Re: [Xen-devel] [PATCH v2 10/12] xenctx: change is_kernel_text() into is_kernel_addr().

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.11.13 at 21:08, Don Slutz <dslutz@xxxxxxxxxxx> wrote:
> @@ -76,22 +86,28 @@ unsigned long long kernel_start = 0xc0000000;
>  unsigned long long kernel_start = 0xffffffff80000000UL;
>  #endif
>  
> -static int is_kernel_text(guest_word_t addr)
> +static type_of_addr is_kernel_addr(guest_word_t addr)

The "is_" prefix is now bogus.

>  {
> -    if (symbol_table == NULL)
> -        return (addr > kernel_start);
> +    if (symbol_table == NULL) {
> +        if (addr > kernel_start)
> +            return KERNEL_TEXT_ADDR;
> +        else
> +            return NOT_KERNEL_ADDR;
> +    }
>  
>      if (addr >= kernel_stext &&
>          addr <= kernel_etext)
> -        return 1;
> -    if (kernel_hypercallpage &&
> -        (addr >= kernel_hypercallpage &&
> -         addr <= kernel_hypercallpage + 4096))
> -        return 1;
> +        return KERNEL_TEXT_ADDR;
> +    if (kernel_hypercallpage && (addr >= kernel_hypercallpage &&
> +                                 addr <= kernel_hypercallpage + 4096))

This reformatting is pointlessly bloating the patch - if you want
the line reformatted, you should (a) do this in the patch that
adds the first part of the condition and (b) obey to indentation
rules.

> +        return KERNEL_TEXT_ADDR;
>      if (addr >= kernel_sinittext &&
>          addr <= kernel_einittext)
> -        return 1;
> -    return 0;
> +        return KERNEL_TEXT_ADDR;
> +    if (addr >= kernel_text &&
> +        addr <= kernel_end)
> +        return KERNEL_DATA_ADDR;

As you supposedly filtered out all text ranges before, did you really
mean to compare to kernel_text here (rather than kernel_start)?

> @@ -749,7 +769,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int 
> vcpu, int width, guest
>              void *p = map_page(ctx, vcpu, stack);
>              if (!p)
>                  return -1;
> -            word = read_stack_word(p, width);
> +            word = read_mem_word(ctx, vcpu, stack, width);

How is this change related to the purpose of the patch?

> @@ -818,7 +838,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int 
> vcpu, int width, guest
>              if (xenctx.stack_trace) {
>                  print_stack_word(stack, width);
>                  printf(": |-- ");
> -                print_stack_word(read_stack_word(p, width), width);
> +                print_stack_word(frame, width);

And this one?

> @@ -843,13 +863,13 @@ static int print_stack(vcpu_guest_context_any_t *ctx, 
> int vcpu, int width, guest
>              void *p = map_page(ctx, vcpu, stack);
>              if (!p)
>                  return -1;
> -            word = read_stack_word(p, width);
> -            if (is_kernel_text(word)) {
> +            word = read_mem_word(ctx, vcpu, stack, width);
> +            if (is_kernel_addr(word) >= KERNEL_TEXT_ADDR) {

And one more.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel