Re: [PATCH] ACPI/APEI: fix ERST MOVE_DATA instruction implementation

[Keir Fraser <keir@xen.org>]

On 17/10/2012 12:36, "Jan Beulich" <JBeulich@suse.com> wrote:

> The src_base and dst_base fields in apei_exec_context are physical
> address, so they should be ioremaped before being used in ERST
> MOVE_DATA instruction.
> 
> Reported-by: Javier Martinez Canillas <martinez.javier@gmail.com>
> Reported-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Huang Ying <ying.huang@intel.com>
> 
> Replace use of ioremap() by __acpi_map_table()/set_fixmap(). Fix error
> handling.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Keir Fraser <keir@xen.org>

> ---
> Note that I have no way to test this code other than by having it go
> through the tester, since I have no hardware available to reproduce
> the regression found there (in other words, this is just a blind
> attempt at addressing the problem uncovered by 26060:4fc87c2f31a0).
> 
> --- a/xen/drivers/acpi/apei/erst.c
> +++ b/xen/drivers/acpi/apei/erst.c
> @@ -247,15 +247,64 @@ static int erst_exec_move_data(struct ap
>  {
> int rc;
> u64 offset;
> +#ifdef CONFIG_X86
> + enum fixed_addresses idx;
> +#endif
> + void *src, *dst;
> +
> + /* ioremap does not work in interrupt context */
> + if (in_irq()) {
> +  printk(KERN_WARNING
> +         "MOVE_DATA cannot be used in interrupt context\n");
> +  return -EBUSY;
> + }
>  
> rc = __apei_exec_read_register(entry, &offset);
> if (rc)
> return rc;
> - memmove((void *)(unsigned long)(ctx->dst_base + offset),
> -  (void *)(unsigned long)(ctx->src_base + offset),
> -  ctx->var2);
>  
> - return 0;
> +#ifdef CONFIG_X86
> + switch (ctx->var2) {
> + case 0:
> +  return 0;
> + case 1 ... PAGE_SIZE:
> +  break;
> + default:
> +  printk(KERN_WARNING
> +         "MOVE_DATA cannot be used for %#"PRIx64" bytes of data\n",
> +         ctx->var2);
> +  return -EOPNOTSUPP;
> + }
> +
> + src = __acpi_map_table(ctx->src_base + offset, ctx->var2);
> +#else
> + src = ioremap(ctx->src_base + offset, ctx->var2);
> +#endif
> + if (!src)
> +  return -ENOMEM;
> +
> +#ifdef CONFIG_X86
> + BUILD_BUG_ON(FIX_ACPI_PAGES < 4);
> + idx = virt_to_fix((unsigned long)src + 2 * PAGE_SIZE);
> + offset += ctx->dst_base;
> + dst = (void *)fix_to_virt(idx) + (offset & ~PAGE_MASK);
> + set_fixmap(idx, offset);
> + if (PFN_DOWN(offset) != PFN_DOWN(offset + ctx->var2 - 1)) {
> +  idx = virt_to_fix((unsigned long)dst + PAGE_SIZE);
> +  set_fixmap(idx, offset + PAGE_SIZE);
> + }
> +#else
> + dst = ioremap(ctx->dst_base + offset, ctx->var2);
> +#endif
> + if (dst) {
> +  memmove(dst, src, ctx->var2);
> +  iounmap(dst);
> + } else
> +  rc = -ENOMEM;
> +
> + iounmap(src);
> +
> + return rc;
>  }
>  
>  static struct apei_exec_ins_type erst_ins_type[] = {
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel