* [PATCH] linux-2.6.18/x86: replace order-based range checking of M2P table by linear one
@ 2011-07-25 10:05 Jan Beulich
2011-07-25 14:19 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2011-07-25 10:05 UTC (permalink / raw)
To: xen-devel@lists.xensource.com
[-- Attachment #1: Type: text/plain, Size: 5128 bytes --]
The order-based approach is not only less efficient (requiring a shift
and a compare, typical generated code looking like this
mov eax, [machine_to_phys_order]
mov ecx, eax
shr ebx, cl
test ebx, ebx
jnz ...
whereas a direct check requires just a compare, like in
cmp ebx, [machine_to_phys_nr]
jae ...
), but also slightly dangerous in the 32-on-64 case - the element
address calculation can wrap if the next power of two boundary is
sufficiently far away from the actual upper limit of the table, and
hence can result in user space addresses being accessed (with it being
unknown what may actually be mapped there).
Signed-off-by: Jan Beulich <jbeulich@novell.com>
--- a/arch/i386/mach-xen/setup.c
+++ b/arch/i386/mach-xen/setup.c
@@ -92,13 +92,12 @@ extern void nmi(void);
unsigned long *machine_to_phys_mapping = (void *)MACH2PHYS_VIRT_START;
EXPORT_SYMBOL(machine_to_phys_mapping);
-unsigned int machine_to_phys_order;
-EXPORT_SYMBOL(machine_to_phys_order);
+unsigned long machine_to_phys_nr;
+EXPORT_SYMBOL(machine_to_phys_nr);
void __init pre_setup_arch_hook(void)
{
struct xen_machphys_mapping mapping;
- unsigned long machine_to_phys_nr_ents;
struct xen_platform_parameters pp;
init_mm.pgd = swapper_pg_dir = (pgd_t *)xen_start_info->pt_base;
@@ -112,10 +111,13 @@ void __init pre_setup_arch_hook(void)
if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, &mapping) == 0) {
machine_to_phys_mapping = (unsigned long *)mapping.v_start;
- machine_to_phys_nr_ents = mapping.max_mfn + 1;
+ machine_to_phys_nr = mapping.max_mfn + 1;
} else
- machine_to_phys_nr_ents = MACH2PHYS_NR_ENTRIES;
- machine_to_phys_order = fls(machine_to_phys_nr_ents - 1);
+ machine_to_phys_nr = MACH2PHYS_NR_ENTRIES;
+ if (machine_to_phys_mapping + machine_to_phys_nr
+ < machine_to_phys_mapping)
+ machine_to_phys_nr = (unsigned long *)NULL
+ - machine_to_phys_mapping;
if (!xen_feature(XENFEAT_auto_translated_physmap))
phys_to_machine_mapping =
--- a/arch/x86_64/kernel/head64-xen.c
+++ b/arch/x86_64/kernel/head64-xen.c
@@ -92,13 +92,12 @@ static void __init setup_boot_cpu_data(v
#include <xen/interface/memory.h>
unsigned long *machine_to_phys_mapping;
EXPORT_SYMBOL(machine_to_phys_mapping);
-unsigned int machine_to_phys_order;
-EXPORT_SYMBOL(machine_to_phys_order);
+unsigned long machine_to_phys_nr;
+EXPORT_SYMBOL(machine_to_phys_nr);
void __init x86_64_start_kernel(char * real_mode_data)
{
struct xen_machphys_mapping mapping;
- unsigned long machine_to_phys_nr_ents;
char *s;
int i;
@@ -112,13 +111,11 @@ void __init x86_64_start_kernel(char * r
xen_start_info->nr_pt_frames;
machine_to_phys_mapping = (unsigned long *)MACH2PHYS_VIRT_START;
- machine_to_phys_nr_ents = MACH2PHYS_NR_ENTRIES;
+ machine_to_phys_nr = MACH2PHYS_NR_ENTRIES;
if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, &mapping) == 0) {
machine_to_phys_mapping = (unsigned long *)mapping.v_start;
- machine_to_phys_nr_ents = mapping.max_mfn + 1;
+ machine_to_phys_nr = mapping.max_mfn + 1;
}
- while ((1UL << machine_to_phys_order) < machine_to_phys_nr_ents )
- machine_to_phys_order++;
#if 0
for (i = 0; i < 256; i++)
--- a/arch/x86_64/mm/init-xen.c
+++ b/arch/x86_64/mm/init-xen.c
@@ -1155,7 +1155,7 @@ int kern_addr_valid(unsigned long addr)
*/
if (addr >= (unsigned long)machine_to_phys_mapping &&
addr < (unsigned long)(machine_to_phys_mapping +
- (1UL << machine_to_phys_order)))
+ machine_to_phys_nr))
return 1;
if (addr >= HYPERVISOR_VIRT_START && addr < HYPERVISOR_VIRT_END)
return 0;
--- a/include/asm-i386/mach-xen/asm/maddr.h
+++ b/include/asm-i386/mach-xen/asm/maddr.h
@@ -25,7 +25,7 @@ extern unsigned long max_mapnr;
#undef machine_to_phys_mapping
extern unsigned long *machine_to_phys_mapping;
-extern unsigned int machine_to_phys_order;
+extern unsigned long machine_to_phys_nr;
static inline unsigned long pfn_to_mfn(unsigned long pfn)
{
@@ -50,7 +50,7 @@ static inline unsigned long mfn_to_pfn(u
if (xen_feature(XENFEAT_auto_translated_physmap))
return mfn;
- if (unlikely((mfn >> machine_to_phys_order) != 0))
+ if (unlikely(mfn >= machine_to_phys_nr))
return max_mapnr;
/* The array access can fail (e.g., device space beyond end of RAM). */
--- a/include/asm-x86_64/mach-xen/asm/maddr.h
+++ b/include/asm-x86_64/mach-xen/asm/maddr.h
@@ -19,7 +19,7 @@ extern unsigned long *phys_to_machine_ma
#undef machine_to_phys_mapping
extern unsigned long *machine_to_phys_mapping;
-extern unsigned int machine_to_phys_order;
+extern unsigned long machine_to_phys_nr;
static inline unsigned long pfn_to_mfn(unsigned long pfn)
{
@@ -44,7 +44,7 @@ static inline unsigned long mfn_to_pfn(u
if (xen_feature(XENFEAT_auto_translated_physmap))
return mfn;
- if (unlikely((mfn >> machine_to_phys_order) != 0))
+ if (unlikely(mfn >= machine_to_phys_nr))
return end_pfn;
/* The array access can fail (e.g., device space beyond end of RAM). */
[-- Attachment #2: xen-x86-m2p-nr.patch --]
[-- Type: text/plain, Size: 5124 bytes --]
The order-based approach is not only less efficient (requiring a shift
and a compare, typical generated code looking like this
mov eax, [machine_to_phys_order]
mov ecx, eax
shr ebx, cl
test ebx, ebx
jnz ...
whereas a direct check requires just a compare, like in
cmp ebx, [machine_to_phys_nr]
jae ...
), but also slightly dangerous in the 32-on-64 case - the element
address calculation can wrap if the next power of two boundary is
sufficiently far away from the actual upper limit of the table, and
hence can result in user space addresses being accessed (with it being
unknown what may actually be mapped there).
Signed-off-by: Jan Beulich <jbeulich@novell.com>
--- a/arch/i386/mach-xen/setup.c
+++ b/arch/i386/mach-xen/setup.c
@@ -92,13 +92,12 @@ extern void nmi(void);
unsigned long *machine_to_phys_mapping = (void *)MACH2PHYS_VIRT_START;
EXPORT_SYMBOL(machine_to_phys_mapping);
-unsigned int machine_to_phys_order;
-EXPORT_SYMBOL(machine_to_phys_order);
+unsigned long machine_to_phys_nr;
+EXPORT_SYMBOL(machine_to_phys_nr);
void __init pre_setup_arch_hook(void)
{
struct xen_machphys_mapping mapping;
- unsigned long machine_to_phys_nr_ents;
struct xen_platform_parameters pp;
init_mm.pgd = swapper_pg_dir = (pgd_t *)xen_start_info->pt_base;
@@ -112,10 +111,13 @@ void __init pre_setup_arch_hook(void)
if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, &mapping) == 0) {
machine_to_phys_mapping = (unsigned long *)mapping.v_start;
- machine_to_phys_nr_ents = mapping.max_mfn + 1;
+ machine_to_phys_nr = mapping.max_mfn + 1;
} else
- machine_to_phys_nr_ents = MACH2PHYS_NR_ENTRIES;
- machine_to_phys_order = fls(machine_to_phys_nr_ents - 1);
+ machine_to_phys_nr = MACH2PHYS_NR_ENTRIES;
+ if (machine_to_phys_mapping + machine_to_phys_nr
+ < machine_to_phys_mapping)
+ machine_to_phys_nr = (unsigned long *)NULL
+ - machine_to_phys_mapping;
if (!xen_feature(XENFEAT_auto_translated_physmap))
phys_to_machine_mapping =
--- a/arch/x86_64/kernel/head64-xen.c
+++ b/arch/x86_64/kernel/head64-xen.c
@@ -92,13 +92,12 @@ static void __init setup_boot_cpu_data(v
#include <xen/interface/memory.h>
unsigned long *machine_to_phys_mapping;
EXPORT_SYMBOL(machine_to_phys_mapping);
-unsigned int machine_to_phys_order;
-EXPORT_SYMBOL(machine_to_phys_order);
+unsigned long machine_to_phys_nr;
+EXPORT_SYMBOL(machine_to_phys_nr);
void __init x86_64_start_kernel(char * real_mode_data)
{
struct xen_machphys_mapping mapping;
- unsigned long machine_to_phys_nr_ents;
char *s;
int i;
@@ -112,13 +111,11 @@ void __init x86_64_start_kernel(char * r
xen_start_info->nr_pt_frames;
machine_to_phys_mapping = (unsigned long *)MACH2PHYS_VIRT_START;
- machine_to_phys_nr_ents = MACH2PHYS_NR_ENTRIES;
+ machine_to_phys_nr = MACH2PHYS_NR_ENTRIES;
if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, &mapping) == 0) {
machine_to_phys_mapping = (unsigned long *)mapping.v_start;
- machine_to_phys_nr_ents = mapping.max_mfn + 1;
+ machine_to_phys_nr = mapping.max_mfn + 1;
}
- while ((1UL << machine_to_phys_order) < machine_to_phys_nr_ents )
- machine_to_phys_order++;
#if 0
for (i = 0; i < 256; i++)
--- a/arch/x86_64/mm/init-xen.c
+++ b/arch/x86_64/mm/init-xen.c
@@ -1155,7 +1155,7 @@ int kern_addr_valid(unsigned long addr)
*/
if (addr >= (unsigned long)machine_to_phys_mapping &&
addr < (unsigned long)(machine_to_phys_mapping +
- (1UL << machine_to_phys_order)))
+ machine_to_phys_nr))
return 1;
if (addr >= HYPERVISOR_VIRT_START && addr < HYPERVISOR_VIRT_END)
return 0;
--- a/include/asm-i386/mach-xen/asm/maddr.h
+++ b/include/asm-i386/mach-xen/asm/maddr.h
@@ -25,7 +25,7 @@ extern unsigned long max_mapnr;
#undef machine_to_phys_mapping
extern unsigned long *machine_to_phys_mapping;
-extern unsigned int machine_to_phys_order;
+extern unsigned long machine_to_phys_nr;
static inline unsigned long pfn_to_mfn(unsigned long pfn)
{
@@ -50,7 +50,7 @@ static inline unsigned long mfn_to_pfn(u
if (xen_feature(XENFEAT_auto_translated_physmap))
return mfn;
- if (unlikely((mfn >> machine_to_phys_order) != 0))
+ if (unlikely(mfn >= machine_to_phys_nr))
return max_mapnr;
/* The array access can fail (e.g., device space beyond end of RAM). */
--- a/include/asm-x86_64/mach-xen/asm/maddr.h
+++ b/include/asm-x86_64/mach-xen/asm/maddr.h
@@ -19,7 +19,7 @@ extern unsigned long *phys_to_machine_ma
#undef machine_to_phys_mapping
extern unsigned long *machine_to_phys_mapping;
-extern unsigned int machine_to_phys_order;
+extern unsigned long machine_to_phys_nr;
static inline unsigned long pfn_to_mfn(unsigned long pfn)
{
@@ -44,7 +44,7 @@ static inline unsigned long mfn_to_pfn(u
if (xen_feature(XENFEAT_auto_translated_physmap))
return mfn;
- if (unlikely((mfn >> machine_to_phys_order) != 0))
+ if (unlikely(mfn >= machine_to_phys_nr))
return end_pfn;
/* The array access can fail (e.g., device space beyond end of RAM). */
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] linux-2.6.18/x86: replace order-based range checking of M2P table by linear one
2011-07-25 10:05 [PATCH] linux-2.6.18/x86: replace order-based range checking of M2P table by linear one Jan Beulich
@ 2011-07-25 14:19 ` Konrad Rzeszutek Wilk
2011-07-25 14:23 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2011-07-25 14:19 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel@lists.xensource.com
On Mon, Jul 25, 2011 at 11:05:22AM +0100, Jan Beulich wrote:
> The order-based approach is not only less efficient (requiring a shift
> and a compare, typical generated code looking like this
>
> mov eax, [machine_to_phys_order]
> mov ecx, eax
> shr ebx, cl
> test ebx, ebx
> jnz ...
>
> whereas a direct check requires just a compare, like in
>
> cmp ebx, [machine_to_phys_nr]
> jae ...
>
> ), but also slightly dangerous in the 32-on-64 case - the element
> address calculation can wrap if the next power of two boundary is
> sufficiently far away from the actual upper limit of the table, and
> hence can result in user space addresses being accessed (with it being
> unknown what may actually be mapped there).
You wouldn't have a patch for upstream Linux for this?
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
> --- a/arch/i386/mach-xen/setup.c
> +++ b/arch/i386/mach-xen/setup.c
> @@ -92,13 +92,12 @@ extern void nmi(void);
>
> unsigned long *machine_to_phys_mapping = (void *)MACH2PHYS_VIRT_START;
> EXPORT_SYMBOL(machine_to_phys_mapping);
> -unsigned int machine_to_phys_order;
> -EXPORT_SYMBOL(machine_to_phys_order);
> +unsigned long machine_to_phys_nr;
> +EXPORT_SYMBOL(machine_to_phys_nr);
>
> void __init pre_setup_arch_hook(void)
> {
> struct xen_machphys_mapping mapping;
> - unsigned long machine_to_phys_nr_ents;
> struct xen_platform_parameters pp;
>
> init_mm.pgd = swapper_pg_dir = (pgd_t *)xen_start_info->pt_base;
> @@ -112,10 +111,13 @@ void __init pre_setup_arch_hook(void)
>
> if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, &mapping) == 0) {
> machine_to_phys_mapping = (unsigned long *)mapping.v_start;
> - machine_to_phys_nr_ents = mapping.max_mfn + 1;
> + machine_to_phys_nr = mapping.max_mfn + 1;
> } else
> - machine_to_phys_nr_ents = MACH2PHYS_NR_ENTRIES;
> - machine_to_phys_order = fls(machine_to_phys_nr_ents - 1);
> + machine_to_phys_nr = MACH2PHYS_NR_ENTRIES;
> + if (machine_to_phys_mapping + machine_to_phys_nr
> + < machine_to_phys_mapping)
> + machine_to_phys_nr = (unsigned long *)NULL
> + - machine_to_phys_mapping;
>
> if (!xen_feature(XENFEAT_auto_translated_physmap))
> phys_to_machine_mapping =
> --- a/arch/x86_64/kernel/head64-xen.c
> +++ b/arch/x86_64/kernel/head64-xen.c
> @@ -92,13 +92,12 @@ static void __init setup_boot_cpu_data(v
> #include <xen/interface/memory.h>
> unsigned long *machine_to_phys_mapping;
> EXPORT_SYMBOL(machine_to_phys_mapping);
> -unsigned int machine_to_phys_order;
> -EXPORT_SYMBOL(machine_to_phys_order);
> +unsigned long machine_to_phys_nr;
> +EXPORT_SYMBOL(machine_to_phys_nr);
>
> void __init x86_64_start_kernel(char * real_mode_data)
> {
> struct xen_machphys_mapping mapping;
> - unsigned long machine_to_phys_nr_ents;
> char *s;
> int i;
>
> @@ -112,13 +111,11 @@ void __init x86_64_start_kernel(char * r
> xen_start_info->nr_pt_frames;
>
> machine_to_phys_mapping = (unsigned long *)MACH2PHYS_VIRT_START;
> - machine_to_phys_nr_ents = MACH2PHYS_NR_ENTRIES;
> + machine_to_phys_nr = MACH2PHYS_NR_ENTRIES;
> if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, &mapping) == 0) {
> machine_to_phys_mapping = (unsigned long *)mapping.v_start;
> - machine_to_phys_nr_ents = mapping.max_mfn + 1;
> + machine_to_phys_nr = mapping.max_mfn + 1;
> }
> - while ((1UL << machine_to_phys_order) < machine_to_phys_nr_ents )
> - machine_to_phys_order++;
>
> #if 0
> for (i = 0; i < 256; i++)
> --- a/arch/x86_64/mm/init-xen.c
> +++ b/arch/x86_64/mm/init-xen.c
> @@ -1155,7 +1155,7 @@ int kern_addr_valid(unsigned long addr)
> */
> if (addr >= (unsigned long)machine_to_phys_mapping &&
> addr < (unsigned long)(machine_to_phys_mapping +
> - (1UL << machine_to_phys_order)))
> + machine_to_phys_nr))
> return 1;
> if (addr >= HYPERVISOR_VIRT_START && addr < HYPERVISOR_VIRT_END)
> return 0;
> --- a/include/asm-i386/mach-xen/asm/maddr.h
> +++ b/include/asm-i386/mach-xen/asm/maddr.h
> @@ -25,7 +25,7 @@ extern unsigned long max_mapnr;
>
> #undef machine_to_phys_mapping
> extern unsigned long *machine_to_phys_mapping;
> -extern unsigned int machine_to_phys_order;
> +extern unsigned long machine_to_phys_nr;
>
> static inline unsigned long pfn_to_mfn(unsigned long pfn)
> {
> @@ -50,7 +50,7 @@ static inline unsigned long mfn_to_pfn(u
> if (xen_feature(XENFEAT_auto_translated_physmap))
> return mfn;
>
> - if (unlikely((mfn >> machine_to_phys_order) != 0))
> + if (unlikely(mfn >= machine_to_phys_nr))
> return max_mapnr;
>
> /* The array access can fail (e.g., device space beyond end of RAM). */
> --- a/include/asm-x86_64/mach-xen/asm/maddr.h
> +++ b/include/asm-x86_64/mach-xen/asm/maddr.h
> @@ -19,7 +19,7 @@ extern unsigned long *phys_to_machine_ma
>
> #undef machine_to_phys_mapping
> extern unsigned long *machine_to_phys_mapping;
> -extern unsigned int machine_to_phys_order;
> +extern unsigned long machine_to_phys_nr;
>
> static inline unsigned long pfn_to_mfn(unsigned long pfn)
> {
> @@ -44,7 +44,7 @@ static inline unsigned long mfn_to_pfn(u
> if (xen_feature(XENFEAT_auto_translated_physmap))
> return mfn;
>
> - if (unlikely((mfn >> machine_to_phys_order) != 0))
> + if (unlikely(mfn >= machine_to_phys_nr))
> return end_pfn;
>
> /* The array access can fail (e.g., device space beyond end of RAM). */
>
>
> The order-based approach is not only less efficient (requiring a shift
> and a compare, typical generated code looking like this
>
> mov eax, [machine_to_phys_order]
> mov ecx, eax
> shr ebx, cl
> test ebx, ebx
> jnz ...
>
> whereas a direct check requires just a compare, like in
>
> cmp ebx, [machine_to_phys_nr]
> jae ...
>
> ), but also slightly dangerous in the 32-on-64 case - the element
> address calculation can wrap if the next power of two boundary is
> sufficiently far away from the actual upper limit of the table, and
> hence can result in user space addresses being accessed (with it being
> unknown what may actually be mapped there).
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
> --- a/arch/i386/mach-xen/setup.c
> +++ b/arch/i386/mach-xen/setup.c
> @@ -92,13 +92,12 @@ extern void nmi(void);
>
> unsigned long *machine_to_phys_mapping = (void *)MACH2PHYS_VIRT_START;
> EXPORT_SYMBOL(machine_to_phys_mapping);
> -unsigned int machine_to_phys_order;
> -EXPORT_SYMBOL(machine_to_phys_order);
> +unsigned long machine_to_phys_nr;
> +EXPORT_SYMBOL(machine_to_phys_nr);
>
> void __init pre_setup_arch_hook(void)
> {
> struct xen_machphys_mapping mapping;
> - unsigned long machine_to_phys_nr_ents;
> struct xen_platform_parameters pp;
>
> init_mm.pgd = swapper_pg_dir = (pgd_t *)xen_start_info->pt_base;
> @@ -112,10 +111,13 @@ void __init pre_setup_arch_hook(void)
>
> if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, &mapping) == 0) {
> machine_to_phys_mapping = (unsigned long *)mapping.v_start;
> - machine_to_phys_nr_ents = mapping.max_mfn + 1;
> + machine_to_phys_nr = mapping.max_mfn + 1;
> } else
> - machine_to_phys_nr_ents = MACH2PHYS_NR_ENTRIES;
> - machine_to_phys_order = fls(machine_to_phys_nr_ents - 1);
> + machine_to_phys_nr = MACH2PHYS_NR_ENTRIES;
> + if (machine_to_phys_mapping + machine_to_phys_nr
> + < machine_to_phys_mapping)
> + machine_to_phys_nr = (unsigned long *)NULL
> + - machine_to_phys_mapping;
>
> if (!xen_feature(XENFEAT_auto_translated_physmap))
> phys_to_machine_mapping =
> --- a/arch/x86_64/kernel/head64-xen.c
> +++ b/arch/x86_64/kernel/head64-xen.c
> @@ -92,13 +92,12 @@ static void __init setup_boot_cpu_data(v
> #include <xen/interface/memory.h>
> unsigned long *machine_to_phys_mapping;
> EXPORT_SYMBOL(machine_to_phys_mapping);
> -unsigned int machine_to_phys_order;
> -EXPORT_SYMBOL(machine_to_phys_order);
> +unsigned long machine_to_phys_nr;
> +EXPORT_SYMBOL(machine_to_phys_nr);
>
> void __init x86_64_start_kernel(char * real_mode_data)
> {
> struct xen_machphys_mapping mapping;
> - unsigned long machine_to_phys_nr_ents;
> char *s;
> int i;
>
> @@ -112,13 +111,11 @@ void __init x86_64_start_kernel(char * r
> xen_start_info->nr_pt_frames;
>
> machine_to_phys_mapping = (unsigned long *)MACH2PHYS_VIRT_START;
> - machine_to_phys_nr_ents = MACH2PHYS_NR_ENTRIES;
> + machine_to_phys_nr = MACH2PHYS_NR_ENTRIES;
> if (HYPERVISOR_memory_op(XENMEM_machphys_mapping, &mapping) == 0) {
> machine_to_phys_mapping = (unsigned long *)mapping.v_start;
> - machine_to_phys_nr_ents = mapping.max_mfn + 1;
> + machine_to_phys_nr = mapping.max_mfn + 1;
> }
> - while ((1UL << machine_to_phys_order) < machine_to_phys_nr_ents )
> - machine_to_phys_order++;
>
> #if 0
> for (i = 0; i < 256; i++)
> --- a/arch/x86_64/mm/init-xen.c
> +++ b/arch/x86_64/mm/init-xen.c
> @@ -1155,7 +1155,7 @@ int kern_addr_valid(unsigned long addr)
> */
> if (addr >= (unsigned long)machine_to_phys_mapping &&
> addr < (unsigned long)(machine_to_phys_mapping +
> - (1UL << machine_to_phys_order)))
> + machine_to_phys_nr))
> return 1;
> if (addr >= HYPERVISOR_VIRT_START && addr < HYPERVISOR_VIRT_END)
> return 0;
> --- a/include/asm-i386/mach-xen/asm/maddr.h
> +++ b/include/asm-i386/mach-xen/asm/maddr.h
> @@ -25,7 +25,7 @@ extern unsigned long max_mapnr;
>
> #undef machine_to_phys_mapping
> extern unsigned long *machine_to_phys_mapping;
> -extern unsigned int machine_to_phys_order;
> +extern unsigned long machine_to_phys_nr;
>
> static inline unsigned long pfn_to_mfn(unsigned long pfn)
> {
> @@ -50,7 +50,7 @@ static inline unsigned long mfn_to_pfn(u
> if (xen_feature(XENFEAT_auto_translated_physmap))
> return mfn;
>
> - if (unlikely((mfn >> machine_to_phys_order) != 0))
> + if (unlikely(mfn >= machine_to_phys_nr))
> return max_mapnr;
>
> /* The array access can fail (e.g., device space beyond end of RAM). */
> --- a/include/asm-x86_64/mach-xen/asm/maddr.h
> +++ b/include/asm-x86_64/mach-xen/asm/maddr.h
> @@ -19,7 +19,7 @@ extern unsigned long *phys_to_machine_ma
>
> #undef machine_to_phys_mapping
> extern unsigned long *machine_to_phys_mapping;
> -extern unsigned int machine_to_phys_order;
> +extern unsigned long machine_to_phys_nr;
>
> static inline unsigned long pfn_to_mfn(unsigned long pfn)
> {
> @@ -44,7 +44,7 @@ static inline unsigned long mfn_to_pfn(u
> if (xen_feature(XENFEAT_auto_translated_physmap))
> return mfn;
>
> - if (unlikely((mfn >> machine_to_phys_order) != 0))
> + if (unlikely(mfn >= machine_to_phys_nr))
> return end_pfn;
>
> /* The array access can fail (e.g., device space beyond end of RAM). */
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] linux-2.6.18/x86: replace order-based range checking of M2P table by linear one
2011-07-25 14:19 ` Konrad Rzeszutek Wilk
@ 2011-07-25 14:23 ` Jan Beulich
2011-07-25 14:33 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2011-07-25 14:23 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: xen-devel@lists.xensource.com
>>> On 25.07.11 at 16:19, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> On Mon, Jul 25, 2011 at 11:05:22AM +0100, Jan Beulich wrote:
>> The order-based approach is not only less efficient (requiring a shift
>> and a compare, typical generated code looking like this
>>
>> mov eax, [machine_to_phys_order]
>> mov ecx, eax
>> shr ebx, cl
>> test ebx, ebx
>> jnz ...
>>
>> whereas a direct check requires just a compare, like in
>>
>> cmp ebx, [machine_to_phys_nr]
>> jae ...
>>
>> ), but also slightly dangerous in the 32-on-64 case - the element
>> address calculation can wrap if the next power of two boundary is
>> sufficiently far away from the actual upper limit of the table, and
>> hence can result in user space addresses being accessed (with it being
>> unknown what may actually be mapped there).
>
> You wouldn't have a patch for upstream Linux for this?
I can try to port this over, but it'll take some time until I can get to
this (certainly not before returning from the summit).
Jan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] linux-2.6.18/x86: replace order-based range checking of M2P table by linear one
2011-07-25 14:23 ` Jan Beulich
@ 2011-07-25 14:33 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2011-07-25 14:33 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel@lists.xensource.com
On Mon, Jul 25, 2011 at 03:23:02PM +0100, Jan Beulich wrote:
> >>> On 25.07.11 at 16:19, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > On Mon, Jul 25, 2011 at 11:05:22AM +0100, Jan Beulich wrote:
> >> The order-based approach is not only less efficient (requiring a shift
> >> and a compare, typical generated code looking like this
> >>
> >> mov eax, [machine_to_phys_order]
> >> mov ecx, eax
> >> shr ebx, cl
> >> test ebx, ebx
> >> jnz ...
> >>
> >> whereas a direct check requires just a compare, like in
> >>
> >> cmp ebx, [machine_to_phys_nr]
> >> jae ...
> >>
> >> ), but also slightly dangerous in the 32-on-64 case - the element
> >> address calculation can wrap if the next power of two boundary is
> >> sufficiently far away from the actual upper limit of the table, and
> >> hence can result in user space addresses being accessed (with it being
> >> unknown what may actually be mapped there).
> >
> > You wouldn't have a patch for upstream Linux for this?
>
> I can try to port this over, but it'll take some time until I can get to
> this (certainly not before returning from the summit).
<nods>Absolutly. Will wait.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-07-25 14:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-25 10:05 [PATCH] linux-2.6.18/x86: replace order-based range checking of M2P table by linear one Jan Beulich
2011-07-25 14:19 ` Konrad Rzeszutek Wilk
2011-07-25 14:23 ` Jan Beulich
2011-07-25 14:33 ` Konrad Rzeszutek Wilk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.