Re: [discuss] [patch] mtrr: fix issues with large addresses
"Joerg Roedel" <[EMAIL PROTECTED]> writes: > On Tue, Feb 06, 2007 at 12:08:12PM -0700, [EMAIL PROTECTED] wrote: >> "Andreas Herrmann" <[EMAIL PROTECTED]> writes: >> > You are referring to current Linux implementation? >> > The AMD64 architecture increased physical address size in PSE mode to >> > 40 bits. So at least it would be possible to use more than 32 bits. >> >> How do you get 40 physical bits in a 32bit page table entry? My memory >> is that the low bits in the page table entry were well defined and >> accounted for. I'm pretty certain I can account for 6 of the low bits >> off the top of my head. PSE is the page size extension allowing pages >> 2MB/4MB >> pages. > > The access to 40 physical address bits is only possible using large pages > (4MB on 32bit without PAE). In those page tables entrys you only use > bits 22:31 for encoding the physical address. The bits 12:21 are > unused. These unused bits are reused to encode bits 32:39 of the 40 bit > physical address. Yep. I missed that feature, and I do see it in AMD documentation now that I look. I'm not certain what that would be useful for though. I'm pretty certain doesn't use this feature, we just enable PAE mode. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [discuss] [patch] mtrr: fix issues with large addresses
On Tue, Feb 06, 2007 at 12:08:12PM -0700, [EMAIL PROTECTED] wrote: > "Andreas Herrmann" <[EMAIL PROTECTED]> writes: > > You are referring to current Linux implementation? > > The AMD64 architecture increased physical address size in PSE mode to > > 40 bits. So at least it would be possible to use more than 32 bits. > > How do you get 40 physical bits in a 32bit page table entry? My memory > is that the low bits in the page table entry were well defined and > accounted for. I'm pretty certain I can account for 6 of the low bits > off the top of my head. PSE is the page size extension allowing pages 2MB/4MB > pages. The access to 40 physical address bits is only possible using large pages (4MB on 32bit without PAE). In those page tables entrys you only use bits 22:31 for encoding the physical address. The bits 12:21 are unused. These unused bits are reused to encode bits 32:39 of the 40 bit physical address. Joerg -- Joerg Roedel Operating System Research Center AMD Saxony LLC & Co. KG - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [discuss] [patch] mtrr: fix issues with large addresses
"Andreas Herrmann" <[EMAIL PROTECTED]> writes: > On Tue, Feb 06, 2007 at 10:54:23AM -0700, [EMAIL PROTECTED] wrote: >> "Andreas Herrmann" <[EMAIL PROTECTED]> writes: >> > On Mon, Feb 05, 2007 at 05:26:12PM -0700, [EMAIL PROTECTED] wrote: >> >> "Andreas Herrmann" <[EMAIL PROTECTED]> writes: >> >> > >> >> The limit is per cpu not per architecture. So if you run a >> >> cpu that can run in 64bit mode in 32bit mode the limit >> >> is not 36 bits. Even PAE in 32bit mode doesn't have that limit. >> >> >> > Good point. >> > >> > I totally ignored that on 64 bit cpus in legacy mode >> > - PAE-paging means up to 52 physical address bits respectively >> > "physical address size of the underlying implementation" >> > - for non-PAE-paging with PSE enabled we have 40 bits for AMD and >> > with PSE36 36 bits for Intel >> >> For non PAE-paging you have 32bits. > > You are referring to current Linux implementation? > The AMD64 architecture increased physical address size in PSE mode to > 40 bits. So at least it would be possible to use more than 32 bits. How do you get 40 physical bits in a 32bit page table entry? My memory is that the low bits in the page table entry were well defined and accounted for. I'm pretty certain I can account for 6 of the low bits off the top of my head. PSE is the page size extension allowing pages 2MB/4MB pages. PAE (physical address extension) gives you a 64bit page table entry and where you have a place for all of those extra physical bits kick as I recall. The limit is 52 bits and current cpus talk about support 40 bits with AMD in the process of going to 48 bits. Is there a feature I have overlooked? That would allow 40 bits with PSE? >> >> Yes. So base needs to be come a u64. > > I was afraid you'ld say that. > >> So base = ((base_hi << 32) | base_lo) >> PAGE_SHIFT. >> >> I see where the 44bit limit comes in. Do you actually have boxes >> with > 16TB? > > No, I don't have access to such a box. Would be nice though. > >> >> Regardless it looks like base and possibly size needs to become >> a u64. At which time the extra >> PAGE_SHIFT could be meaningless. >> Either that or because base and size need to be sized in something like >> megabytes. >> >> I suspect making it a u64 sized in bytes will get the job done and >> result in simpler code. > > Right you are! > Ok, it is best to do (3). > I will come up with another patch asap. Thanks. Sorry for being a pain. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [discuss] [patch] mtrr: fix issues with large addresses
On Tue, Feb 06, 2007 at 10:54:23AM -0700, [EMAIL PROTECTED] wrote: > "Andreas Herrmann" <[EMAIL PROTECTED]> writes: > > On Mon, Feb 05, 2007 at 05:26:12PM -0700, [EMAIL PROTECTED] wrote: > >> "Andreas Herrmann" <[EMAIL PROTECTED]> writes: > >> > > >> The limit is per cpu not per architecture. So if you run a > >> cpu that can run in 64bit mode in 32bit mode the limit > >> is not 36 bits. Even PAE in 32bit mode doesn't have that limit. > >> > > Good point. > > > > I totally ignored that on 64 bit cpus in legacy mode > > - PAE-paging means up to 52 physical address bits respectively > > "physical address size of the underlying implementation" > > - for non-PAE-paging with PSE enabled we have 40 bits for AMD and > > with PSE36 36 bits for Intel > > For non PAE-paging you have 32bits. You are referring to current Linux implementation? The AMD64 architecture increased physical address size in PSE mode to 40 bits. So at least it would be possible to use more than 32 bits. > >> > diff --git a/arch/i386/kernel/cpu/mtrr/generic.c > >> > b/arch/i386/kernel/cpu/mtrr/generic.c > >> > index f77fc53..aa21d15 100644 > >> > --- a/arch/i386/kernel/cpu/mtrr/generic.c > >> > +++ b/arch/i386/kernel/cpu/mtrr/generic.c > >> > @@ -172,7 +172,7 @@ int generic_get_free_region(unsigned long base, > >> > unsigned > >> > long size, int replace_ > >> > static void generic_get_mtrr(unsigned int reg, unsigned long *base, > >> > unsigned long *size, mtrr_type *type) > >> > { > >> > -unsigned int mask_lo, mask_hi, base_lo, base_hi; > >> > +unsigned long mask_lo, mask_hi, base_lo, base_hi; > >> > >> Why? Given the low and the high I am assuming these are all implicitly > >> 32bit quantities. unsigned int is fine. > > > > It is not, please refer to the function body, e.g. > > > > *base = base_hi << (32 - PAGE_SHIFT) | base_lo >> PAGE_SHIFT; > > > > All leading 20 bits of "unsigned int" base_hi are snipped away. Thus > > limiting base to use 44 bit instead of 52 bit in 64 bit mode. An > > option would have been to use a type cast while shifting. > > > > (Hmm, having your first remark in mind I have to admit that my fix is > > mainly focused on 64 bit mode not on 64 bit cpu running in 32 bit ...) > > Yes. So base needs to be come a u64. I was afraid you'ld say that. > So base = ((base_hi << 32) | base_lo) >> PAGE_SHIFT. > > I see where the 44bit limit comes in. Do you actually have boxes > with > 16TB? No, I don't have access to such a box. Would be nice though. > > Regardless it looks like base and possibly size needs to become > a u64. At which time the extra >> PAGE_SHIFT could be meaningless. > Either that or because base and size need to be sized in something like > megabytes. > > I suspect making it a u64 sized in bytes will get the job done and > result in simpler code. Right you are! > >> > diff --git a/arch/i386/kernel/cpu/mtrr/if.c > >> > b/arch/i386/kernel/cpu/mtrr/if.c > >> > index 5ae1705..3abc3f1 100644 > >> > --- a/arch/i386/kernel/cpu/mtrr/if.c > >> > +++ b/arch/i386/kernel/cpu/mtrr/if.c > >> > @@ -137,6 +137,10 @@ mtrr_write(struct file *file, const char __user > >> > *buf, > >> > size_t len, loff_t * ppos) > >> > for (i = 0; i < MTRR_NUM_TYPES; ++i) { > >> > if (strcmp(ptr, mtrr_strings[i])) > >> > continue; > >> > +#ifndef CONFIG_X86_64 > >> > +if (base > 0xfULL) > >> > +return -EINVAL; > >> > +#endif > >> > >> That is just silly. If the cpu is running in long mode or should > >> not affect this capability. > > So I could do one of the following: > > (1) prepare new patch omitting this silly hunk (-> old behaviour) > > (2) check for 44 bit address size instead of 36 bit address size to > > reflect the implicit truncation (-> avoid silent truncation) > > (3) fix all mtrr code to be able to use up to 52 bit width physical > > addresses instead of 44 bit ones if running in 32 bit mode on 64 bit > > cpus. > > > > I prefer to do (2). > > (IMHO those who have the need for n>44 bit width base address in an MTRR > > should stick to 64 bit mode.) > > I prefer (3). Since the code is shared between 32 and 64bit mode it > should behave the same in both. I know there are people who regularly > test 32bit kernels on boxes with 128 cpus and 128MB of ram. > > People sometimes want crazy things and since it just a matter of changing > the type it should be no real work to get the code to work in 32bit mode. Ok, it is best to do (3). I will come up with another patch asap. > Eric Regards, Andreas -- AMD Saxony, Dresden, Germany Operating System Research Center - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mtrr: fix issues with large addresses
"Andreas Herrmann" <[EMAIL PROTECTED]> writes: > On Mon, Feb 05, 2007 at 05:26:12PM -0700, [EMAIL PROTECTED] wrote: >> "Andreas Herrmann" <[EMAIL PROTECTED]> writes: >> > mtrr: fix issues with large addresses >> > >> > Fixes some issues with /proc/mtrr interface: >> > o If physical address size crosses the 44 bit boundary >> > size_or_mask is evaluated wrong >> > o size_and_mask limits physical base >> > address for an MTRR to be less than 44 bit >> > o added check to restrict base address to 36 bit on i386 >> >> The limit is per cpu not per architecture. So if you run a >> cpu that can run in 64bit mode in 32bit mode the limit >> is not 36 bits. Even PAE in 32bit mode doesn't have that limit. >> > > Good point. > > I totally ignored that on 64 bit cpus in legacy mode > - PAE-paging means up to 52 physical address bits respectively > "physical address size of the underlying implementation" > - for non-PAE-paging with PSE enabled we have 40 bits for AMD and > with PSE36 36 bits for Intel For non PAE-paging you have 32bits. The real question is how many physical bits does the cpu implemented for bus transactions. The latest Intel (non-ia64) cpus are at 40 bits I believe, and the cutting edge AMD cpus are moving to 48bits. The Intel bus protocol supports 50 bits physical at least in it's ia64 incarnation so I believe some of the chipsets may actually support that. > What a mess. > (Hope anyone knows for sure which paging methods are relevant for > Linux if compiled for i386 and w/o CONFIG_X86_64?) PAE does get used on i386, but except that it is a related phenomenon it isn't relevant. > (Seems that in my mind this legacy stuff is still tied to 36 and 32 > bits :( Basically Intel had 36 bits implemented for so long it just got stuck in everyones heads. Forget that. It is a question of how many physical address bits the cpu uses when generating bus cycles. Nothing about the mode the cpu is running in matters. >> > diff --git a/arch/i386/kernel/cpu/mtrr/generic.c >> > b/arch/i386/kernel/cpu/mtrr/generic.c >> > index f77fc53..aa21d15 100644 >> > --- a/arch/i386/kernel/cpu/mtrr/generic.c >> > +++ b/arch/i386/kernel/cpu/mtrr/generic.c >> > @@ -172,7 +172,7 @@ int generic_get_free_region(unsigned long base, >> > unsigned >> > long size, int replace_ >> > static void generic_get_mtrr(unsigned int reg, unsigned long *base, >> > unsigned long *size, mtrr_type *type) >> > { >> > - unsigned int mask_lo, mask_hi, base_lo, base_hi; >> > + unsigned long mask_lo, mask_hi, base_lo, base_hi; >> >> Why? Given the low and the high I am assuming these are all implicitly >> 32bit quantities. unsigned int is fine. > > It is not, please refer to the function body, e.g. > > *base = base_hi << (32 - PAGE_SHIFT) | base_lo >> PAGE_SHIFT; > > All leading 20 bits of "unsigned int" base_hi are snipped away. Thus > limiting base to use 44 bit instead of 52 bit in 64 bit mode. An > option would have been to use a type cast while shifting. > > (Hmm, having your first remark in mind I have to admit that my fix is > mainly focused on 64 bit mode not on 64 bit cpu running in 32 bit ...) Yes. So base needs to be come a u64. So base = ((base_hi << 32) | base_lo) >> PAGE_SHIFT. I see where the 44bit limit comes in. Do you actually have boxes with > 16TB? Regardless it looks like base and possibly size needs to become a u64. At which time the extra >> PAGE_SHIFT could be meaningless. Either that or because base and size need to be sized in something like megabytes. I suspect making it a u64 sized in bytes will get the job done and result in simpler code. >> > diff --git a/arch/i386/kernel/cpu/mtrr/if.c >> > b/arch/i386/kernel/cpu/mtrr/if.c >> > index 5ae1705..3abc3f1 100644 >> > --- a/arch/i386/kernel/cpu/mtrr/if.c >> > +++ b/arch/i386/kernel/cpu/mtrr/if.c >> > @@ -137,6 +137,10 @@ mtrr_write(struct file *file, const char __user *buf, >> > size_t len, loff_t * ppos) >> >for (i = 0; i < MTRR_NUM_TYPES; ++i) { >> >if (strcmp(ptr, mtrr_strings[i])) >> >continue; >> > +#ifndef CONFIG_X86_64 >> > + if (base > 0xfULL) >> > + return -EINVAL; >> > +#endif >> >> That is just silly. If the cpu is running in long mode or should >> not affect this capability. > > Yes, that check is wrong -- due to my wrong assumption. > Base can use 52 bits not just 36 bits in 32 bit mode on 64 bit cpu. > > My intention was to avoid the silent truncation of base address > in the following lines: > > base >>= PAGE_SHIFT; > size >>= PAGE_SHIFT; > err = > mtrr_add_page((unsigned long) base, ... > > where base is cut at bit 44 due to the type cast. > > The user doing > #> echo 0x1000 size=0x0815000 type=uncachable >/proc/mtrr > will end up with a new MTRR pair having PhysBase==0x0. (At least this > will give him some time to get a coffee when his system reboots after >
Re: [discuss] [patch] mtrr: fix issues with large addresses
On Tue, Feb 06, 2007 at 11:54:57AM +0100, Andi Kleen wrote: > On Tuesday 06 February 2007 10:53, Jan Beulich wrote: > > >> I don't think I remember a restriction here, at least not below 44 bits > > >> (that's where pfn-s would need to become 64-bit wide). > > > > > >The i386 mm code only supports 4 entries in the PGD, so more than 36bit > > >cannot > > >be mapped right now. > > > > That has nothing to do with the number of physical address bits. > > You couldn't use the memory in any ways. > > Anyways I give up -- the check is probably not needed, unless Andreas > comes up with a good reason. No, I haven't a good reason to restrict the base address to fewer than 44 bits. So the question is, should I completely remove that check or adapt it to check for 44 bit instead of 36 bit? Regards, Andreas -- AMD Saxony, Dresden, Germany Operating System Research Center - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [discuss] [patch] mtrr: fix issues with large addresses
On Tue, Feb 06, 2007 at 09:31:45AM +, Jan Beulich wrote: > >>> Andi Kleen <[EMAIL PROTECTED]> 06.02.07 08:53 >>> > >On Monday 05 February 2007 23:50, Siddha, Suresh B wrote: > >> On Mon, Feb 05, 2007 at 06:19:59PM +0100, Andreas Herrmann wrote: > >> > o added check to restrict base address to 36 bit on i386 > >> > >> Why is this? It can go upto implemented physical bits, right? > > > >In theory it can, but Linux doesn't support it. > > I don't think I remember a restriction here, at least not below 44 bits > (that's where pfn-s would need to become 64-bit wide). > > Jan > Hi all, shame on me. Wanted to fix and interface issue where base address is truncated at 44 bit in mtrr_write(). (And just thought 36 bit would be more than enough for that 32 bit Linux version :) Regards, Andreas -- AMD Saxony, Dresden, Germany Operating System Research Center - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mtrr: fix issues with large addresses
On Mon, Feb 05, 2007 at 05:26:12PM -0700, [EMAIL PROTECTED] wrote: > "Andreas Herrmann" <[EMAIL PROTECTED]> writes: > > mtrr: fix issues with large addresses > > > > Fixes some issues with /proc/mtrr interface: > > o If physical address size crosses the 44 bit boundary > > size_or_mask is evaluated wrong > > o size_and_mask limits physical base > > address for an MTRR to be less than 44 bit > > o added check to restrict base address to 36 bit on i386 > > The limit is per cpu not per architecture. So if you run a > cpu that can run in 64bit mode in 32bit mode the limit > is not 36 bits. Even PAE in 32bit mode doesn't have that limit. > Good point. I totally ignored that on 64 bit cpus in legacy mode - PAE-paging means up to 52 physical address bits respectively "physical address size of the underlying implementation" - for non-PAE-paging with PSE enabled we have 40 bits for AMD and with PSE36 36 bits for Intel What a mess. (Hope anyone knows for sure which paging methods are relevant for Linux if compiled for i386 and w/o CONFIG_X86_64?) (Seems that in my mind this legacy stuff is still tied to 36 and 32 bits :( > > diff --git a/arch/i386/kernel/cpu/mtrr/generic.c > > b/arch/i386/kernel/cpu/mtrr/generic.c > > index f77fc53..aa21d15 100644 > > --- a/arch/i386/kernel/cpu/mtrr/generic.c > > +++ b/arch/i386/kernel/cpu/mtrr/generic.c > > @@ -172,7 +172,7 @@ int generic_get_free_region(unsigned long base, unsigned > > long size, int replace_ > > static void generic_get_mtrr(unsigned int reg, unsigned long *base, > > unsigned long *size, mtrr_type *type) > > { > > - unsigned int mask_lo, mask_hi, base_lo, base_hi; > > + unsigned long mask_lo, mask_hi, base_lo, base_hi; > > Why? Given the low and the high I am assuming these are all implicitly > 32bit quantities. unsigned int is fine. It is not, please refer to the function body, e.g. *base = base_hi << (32 - PAGE_SHIFT) | base_lo >> PAGE_SHIFT; All leading 20 bits of "unsigned int" base_hi are snipped away. Thus limiting base to use 44 bit instead of 52 bit in 64 bit mode. An option would have been to use a type cast while shifting. (Hmm, having your first remark in mind I have to admit that my fix is mainly focused on 64 bit mode not on 64 bit cpu running in 32 bit ...) > > diff --git a/arch/i386/kernel/cpu/mtrr/if.c b/arch/i386/kernel/cpu/mtrr/if.c > > index 5ae1705..3abc3f1 100644 > > --- a/arch/i386/kernel/cpu/mtrr/if.c > > +++ b/arch/i386/kernel/cpu/mtrr/if.c > > @@ -137,6 +137,10 @@ mtrr_write(struct file *file, const char __user *buf, > > size_t len, loff_t * ppos) > > for (i = 0; i < MTRR_NUM_TYPES; ++i) { > > if (strcmp(ptr, mtrr_strings[i])) > > continue; > > +#ifndef CONFIG_X86_64 > > + if (base > 0xfULL) > > + return -EINVAL; > > +#endif > > That is just silly. If the cpu is running in long mode or should > not affect this capability. Yes, that check is wrong -- due to my wrong assumption. Base can use 52 bits not just 36 bits in 32 bit mode on 64 bit cpu. My intention was to avoid the silent truncation of base address in the following lines: base >>= PAGE_SHIFT; size >>= PAGE_SHIFT; err = mtrr_add_page((unsigned long) base, ... where base is cut at bit 44 due to the type cast. The user doing #> echo 0x1000 size=0x0815000 type=uncachable >/proc/mtrr will end up with a new MTRR pair having PhysBase==0x0. (At least this will give him some time to get a coffee when his system reboots after the crash.) So it seems that some more stuff needs to be fixed in the mtrr code. All unsigned long base addresses used in this code implicitly restrict the address to 44 bit (taking the PAGE_SHIFT into account). So I could do one of the following: (1) prepare new patch omitting this silly hunk (-> old behaviour) (2) check for 44 bit address size instead of 36 bit address size to reflect the implicit truncation (-> avoid silent truncation) (3) fix all mtrr code to be able to use up to 52 bit width physical addresses instead of 44 bit ones if running in 32 bit mode on 64 bit cpus. I prefer to do (2). (IMHO those who have the need for n>44 bit width base address in an MTRR should stick to 64 bit mode.) > > diff --git a/arch/i386/kernel/cpu/mtrr/main.c > > b/arch/i386/kernel/cpu/mtrr/main.c > > index 16bb7ea..0acfb6a 100644 > > --- a/arch/i386/kernel/cpu/mtrr/main.c > > +++ b/arch/i386/kernel/cpu/mtrr/main.c > > @@ -50,7 +50,7 @@ u32 num_var_ranges = 0; > > unsigned int *usage_table; > > static DEFINE_MUTEX(mtrr_mutex); > > > > -u32 size_or_mask, size_and_mask; > > +u64 size_or_mask, size_and_mask; > > > > static struct mtrr_ops * mtrr_ops[X86_VENDOR_NUM] = {}; > > > > @@ -662,8 +662,8 @@ void __init mtrr_bp_init(void) > > boot_cpu_data.x86_mask == 0x4)) > > phys_addr = 36; > > > > -
Re: [discuss] [patch] mtrr: fix issues with large addresses
On Tuesday 06 February 2007 10:53, Jan Beulich wrote: > >> I don't think I remember a restriction here, at least not below 44 bits > >> (that's where pfn-s would need to become 64-bit wide). > > > >The i386 mm code only supports 4 entries in the PGD, so more than 36bit > >cannot > >be mapped right now. > > That has nothing to do with the number of physical address bits. You couldn't use the memory in any ways. Anyways I give up -- the check is probably not needed, unless Andreas comes up with a good reason. > > >Also even 64MB barely works (many boxes don't boot), you would likely > >need at least the 4:4 patch to go >64GB. Also we know there are tons > >of possible deadlocks in various subsystems when the lowmem:highmem ratio > >gets so out of hand. > > > >Ok it could be probably all fixed with some work (at least the mm part, > >the deadlocks would be more tricky), but would seem fairly > >pointless to me because all machines with >36bits support are 64bit capable. > > That's a different story, and certainly a limiting factor. But this shouldn't > e.g. disallow (hypothetical?) systems that have a very sparse memory map > extending beyond 64G. They would need a discontig kernel to boot most likely, otherwise mem_map would fill up their memory. And I was told Windows doesn't like that, so it's unlikely there will ever be such x86 machines. -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [discuss] [patch] mtrr: fix issues with large addresses
>> I don't think I remember a restriction here, at least not below 44 bits >> (that's where pfn-s would need to become 64-bit wide). > >The i386 mm code only supports 4 entries in the PGD, so more than 36bit cannot >be mapped right now. That has nothing to do with the number of physical address bits. >Also even 64MB barely works (many boxes don't boot), you would likely >need at least the 4:4 patch to go >64GB. Also we know there are tons >of possible deadlocks in various subsystems when the lowmem:highmem ratio >gets so out of hand. > >Ok it could be probably all fixed with some work (at least the mm part, >the deadlocks would be more tricky), but would seem fairly >pointless to me because all machines with >36bits support are 64bit capable. That's a different story, and certainly a limiting factor. But this shouldn't e.g. disallow (hypothetical?) systems that have a very sparse memory map extending beyond 64G. Jan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [discuss] [patch] mtrr: fix issues with large addresses
On Tuesday 06 February 2007 10:31, Jan Beulich wrote: > >>> Andi Kleen <[EMAIL PROTECTED]> 06.02.07 08:53 >>> > >On Monday 05 February 2007 23:50, Siddha, Suresh B wrote: > >> On Mon, Feb 05, 2007 at 06:19:59PM +0100, Andreas Herrmann wrote: > >> > o added check to restrict base address to 36 bit on i386 > >> > >> Why is this? It can go upto implemented physical bits, right? > > > >In theory it can, but Linux doesn't support it. > > I don't think I remember a restriction here, at least not below 44 bits > (that's where pfn-s would need to become 64-bit wide). The i386 mm code only supports 4 entries in the PGD, so more than 36bit cannot be mapped right now. Also even 64MB barely works (many boxes don't boot), you would likely need at least the 4:4 patch to go >64GB. Also we know there are tons of possible deadlocks in various subsystems when the lowmem:highmem ratio gets so out of hand. Ok it could be probably all fixed with some work (at least the mm part, the deadlocks would be more tricky), but would seem fairly pointless to me because all machines with >36bits support are 64bit capable. -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [discuss] [patch] mtrr: fix issues with large addresses
>>> Andi Kleen <[EMAIL PROTECTED]> 06.02.07 08:53 >>> >On Monday 05 February 2007 23:50, Siddha, Suresh B wrote: >> On Mon, Feb 05, 2007 at 06:19:59PM +0100, Andreas Herrmann wrote: >> > o added check to restrict base address to 36 bit on i386 >> >> Why is this? It can go upto implemented physical bits, right? > >In theory it can, but Linux doesn't support it. I don't think I remember a restriction here, at least not below 44 bits (that's where pfn-s would need to become 64-bit wide). Jan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mtrr: fix issues with large addresses
On Monday 05 February 2007 23:50, Siddha, Suresh B wrote: > On Mon, Feb 05, 2007 at 06:19:59PM +0100, Andreas Herrmann wrote: > > o added check to restrict base address to 36 bit on i386 > > Why is this? It can go upto implemented physical bits, right? In theory it can, but Linux doesn't support it. -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mtrr: fix issues with large addresses
"Andreas Herrmann" <[EMAIL PROTECTED]> writes: > Hi, > > This is a repost of a mail sent to Richard Gooch and lkml some time > ago. Meanwhile I noticed that Richard has a new email address. And it > seems that he does not maintain the mtrr code anymore. (So how about > updating the MAINTAINERS file?) > > Here we go again -- with new recipient and a slightly modified > version of the patch. > > > Regards, > > Andreas > > > mtrr: fix issues with large addresses > > Fixes some issues with /proc/mtrr interface: > o If physical address size crosses the 44 bit boundary > size_or_mask is evaluated wrong > o size_and_mask limits physical base > address for an MTRR to be less than 44 bit > o added check to restrict base address to 36 bit on i386 The limit is per cpu not per architecture. So if you run a cpu that can run in 64bit mode in 32bit mode the limit is not 36 bits. Even PAE in 32bit mode doesn't have that limit. > Signed-off-by: Andreas Herrmann <[EMAIL PROTECTED]> > > -- > diff --git a/arch/i386/kernel/cpu/mtrr/generic.c > b/arch/i386/kernel/cpu/mtrr/generic.c > index f77fc53..aa21d15 100644 > --- a/arch/i386/kernel/cpu/mtrr/generic.c > +++ b/arch/i386/kernel/cpu/mtrr/generic.c > @@ -172,7 +172,7 @@ int generic_get_free_region(unsigned long base, unsigned > long size, int replace_ > static void generic_get_mtrr(unsigned int reg, unsigned long *base, >unsigned long *size, mtrr_type *type) > { > - unsigned int mask_lo, mask_hi, base_lo, base_hi; > + unsigned long mask_lo, mask_hi, base_lo, base_hi; Why? Given the low and the high I am assuming these are all implicitly 32bit quantities. unsigned int is fine. > rdmsr(MTRRphysMask_MSR(reg), mask_lo, mask_hi); > if ((mask_lo & 0x800) == 0) { > diff --git a/arch/i386/kernel/cpu/mtrr/if.c b/arch/i386/kernel/cpu/mtrr/if.c > index 5ae1705..3abc3f1 100644 > --- a/arch/i386/kernel/cpu/mtrr/if.c > +++ b/arch/i386/kernel/cpu/mtrr/if.c > @@ -137,6 +137,10 @@ mtrr_write(struct file *file, const char __user *buf, > size_t len, loff_t * ppos) > for (i = 0; i < MTRR_NUM_TYPES; ++i) { > if (strcmp(ptr, mtrr_strings[i])) > continue; > +#ifndef CONFIG_X86_64 > + if (base > 0xfULL) > + return -EINVAL; > +#endif That is just silly. If the cpu is running in long mode or should not affect this capability. > base >>= PAGE_SHIFT; > size >>= PAGE_SHIFT; > err = > diff --git a/arch/i386/kernel/cpu/mtrr/main.c > b/arch/i386/kernel/cpu/mtrr/main.c > index 16bb7ea..0acfb6a 100644 > --- a/arch/i386/kernel/cpu/mtrr/main.c > +++ b/arch/i386/kernel/cpu/mtrr/main.c > @@ -50,7 +50,7 @@ u32 num_var_ranges = 0; > unsigned int *usage_table; > static DEFINE_MUTEX(mtrr_mutex); > > -u32 size_or_mask, size_and_mask; > +u64 size_or_mask, size_and_mask; > > static struct mtrr_ops * mtrr_ops[X86_VENDOR_NUM] = {}; > > @@ -662,8 +662,8 @@ void __init mtrr_bp_init(void) >boot_cpu_data.x86_mask == 0x4)) > phys_addr = 36; > > - size_or_mask = ~((1 << (phys_addr - PAGE_SHIFT)) - 1); > - size_and_mask = ~size_or_mask & 0xfff0; > + size_or_mask = ~((1ULL << (phys_addr - PAGE_SHIFT)) - 1); > + size_and_mask = ~size_or_mask & 0xf0ULL; Don't you want to make this hard coded mask 0xfff0ULL? Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mtrr: fix issues with large addresses
On Mon, Feb 05, 2007 at 06:19:59PM +0100, Andreas Herrmann wrote: > o added check to restrict base address to 36 bit on i386 Why is this? It can go upto implemented physical bits, right? thanks, suresh - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch] mtrr: fix issues with large addresses
Hi, This is a repost of a mail sent to Richard Gooch and lkml some time ago. Meanwhile I noticed that Richard has a new email address. And it seems that he does not maintain the mtrr code anymore. (So how about updating the MAINTAINERS file?) Here we go again -- with new recipient and a slightly modified version of the patch. Regards, Andreas mtrr: fix issues with large addresses Fixes some issues with /proc/mtrr interface: o If physical address size crosses the 44 bit boundary size_or_mask is evaluated wrong o size_and_mask limits physical base address for an MTRR to be less than 44 bit o added check to restrict base address to 36 bit on i386 Signed-off-by: Andreas Herrmann <[EMAIL PROTECTED]> -- diff --git a/arch/i386/kernel/cpu/mtrr/generic.c b/arch/i386/kernel/cpu/mtrr/generic.c index f77fc53..aa21d15 100644 --- a/arch/i386/kernel/cpu/mtrr/generic.c +++ b/arch/i386/kernel/cpu/mtrr/generic.c @@ -172,7 +172,7 @@ int generic_get_free_region(unsigned long base, unsigned long size, int replace_ static void generic_get_mtrr(unsigned int reg, unsigned long *base, unsigned long *size, mtrr_type *type) { - unsigned int mask_lo, mask_hi, base_lo, base_hi; + unsigned long mask_lo, mask_hi, base_lo, base_hi; rdmsr(MTRRphysMask_MSR(reg), mask_lo, mask_hi); if ((mask_lo & 0x800) == 0) { diff --git a/arch/i386/kernel/cpu/mtrr/if.c b/arch/i386/kernel/cpu/mtrr/if.c index 5ae1705..3abc3f1 100644 --- a/arch/i386/kernel/cpu/mtrr/if.c +++ b/arch/i386/kernel/cpu/mtrr/if.c @@ -137,6 +137,10 @@ mtrr_write(struct file *file, const char __user *buf, size_t len, loff_t * ppos) for (i = 0; i < MTRR_NUM_TYPES; ++i) { if (strcmp(ptr, mtrr_strings[i])) continue; +#ifndef CONFIG_X86_64 + if (base > 0xfULL) + return -EINVAL; +#endif base >>= PAGE_SHIFT; size >>= PAGE_SHIFT; err = diff --git a/arch/i386/kernel/cpu/mtrr/main.c b/arch/i386/kernel/cpu/mtrr/main.c index 16bb7ea..0acfb6a 100644 --- a/arch/i386/kernel/cpu/mtrr/main.c +++ b/arch/i386/kernel/cpu/mtrr/main.c @@ -50,7 +50,7 @@ u32 num_var_ranges = 0; unsigned int *usage_table; static DEFINE_MUTEX(mtrr_mutex); -u32 size_or_mask, size_and_mask; +u64 size_or_mask, size_and_mask; static struct mtrr_ops * mtrr_ops[X86_VENDOR_NUM] = {}; @@ -662,8 +662,8 @@ void __init mtrr_bp_init(void) boot_cpu_data.x86_mask == 0x4)) phys_addr = 36; - size_or_mask = ~((1 << (phys_addr - PAGE_SHIFT)) - 1); - size_and_mask = ~size_or_mask & 0xfff0; + size_or_mask = ~((1ULL << (phys_addr - PAGE_SHIFT)) - 1); + size_and_mask = ~size_or_mask & 0xf0ULL; } else if (boot_cpu_data.x86_vendor == X86_VENDOR_CENTAUR && boot_cpu_data.x86 == 6) { /* VIA C* family have Intel style MTRRs, but diff --git a/arch/i386/kernel/cpu/mtrr/mtrr.h b/arch/i386/kernel/cpu/mtrr/mtrr.h index d61ea9d..289dfe6 100644 --- a/arch/i386/kernel/cpu/mtrr/mtrr.h +++ b/arch/i386/kernel/cpu/mtrr/mtrr.h @@ -84,7 +84,7 @@ void get_mtrr_state(void); extern void set_mtrr_ops(struct mtrr_ops * ops); -extern u32 size_or_mask, size_and_mask; +extern u64 size_or_mask, size_and_mask; extern struct mtrr_ops * mtrr_if; #define is_cpu(vnd)(mtrr_if && mtrr_if->vendor == X86_VENDOR_##vnd) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/