* [PATCH] ACPI/APEI: fix ERST MOVE_DATA instruction implementation
@ 2012-10-17 11:36 Jan Beulich
2012-10-17 11:47 ` Keir Fraser
2012-10-17 11:53 ` Jan Beulich
0 siblings, 2 replies; 6+ messages in thread
From: Jan Beulich @ 2012-10-17 11:36 UTC (permalink / raw)
To: xen-devel
[-- Attachment #1: Type: text/plain, Size: 2498 bytes --]
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>
---
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[] = {
[-- Attachment #2: ACPI-ERST-move-data-map.patch --]
[-- Type: text/plain, Size: 2552 bytes --]
ACPI/APEI: fix ERST MOVE_DATA instruction implementation
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>
---
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[] = {
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ACPI/APEI: fix ERST MOVE_DATA instruction implementation
2012-10-17 11:36 [PATCH] ACPI/APEI: fix ERST MOVE_DATA instruction implementation Jan Beulich
@ 2012-10-17 11:47 ` Keir Fraser
2012-10-17 11:53 ` Jan Beulich
1 sibling, 0 replies; 6+ messages in thread
From: Keir Fraser @ 2012-10-17 11:47 UTC (permalink / raw)
To: Jan Beulich, xen-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ACPI/APEI: fix ERST MOVE_DATA instruction implementation
2012-10-17 11:36 [PATCH] ACPI/APEI: fix ERST MOVE_DATA instruction implementation Jan Beulich
2012-10-17 11:47 ` Keir Fraser
@ 2012-10-17 11:53 ` Jan Beulich
2012-10-17 12:07 ` Keir Fraser
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2012-10-17 11:53 UTC (permalink / raw)
To: xen-devel
>>> On 17.10.12 at 13:36, "Jan Beulich" <JBeulich@suse.com> wrote:
> --- 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
Before anyone asks - yes, I have added to my todo list to put in
place a proper ioremap() for x86. But since I'm under some time
pressure right now, this was the faster route to something that
can be committed to see whether the regression goes away.
Jan
> + 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[] = {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ACPI/APEI: fix ERST MOVE_DATA instruction implementation
2012-10-17 11:53 ` Jan Beulich
@ 2012-10-17 12:07 ` Keir Fraser
2012-10-17 12:15 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Keir Fraser @ 2012-10-17 12:07 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 17/10/2012 12:53, "Jan Beulich" <JBeulich@suse.com> wrote:
> Before anyone asks - yes, I have added to my todo list to put in
> place a proper ioremap() for x86. But since I'm under some time
> pressure right now, this was the faster route to something that
> can be committed to see whether the regression goes away.
That's good as we'll need it for >4kB per-VCPU stacks (even if we only need
4kB VCPU stacks, we'll want a guard page too).
-- Keir
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI/APEI: fix ERST MOVE_DATA instruction implementation
2012-10-17 12:07 ` Keir Fraser
@ 2012-10-17 12:15 ` Jan Beulich
2012-10-17 13:01 ` Keir Fraser
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2012-10-17 12:15 UTC (permalink / raw)
To: Keir Fraser; +Cc: xen-devel
>>> On 17.10.12 at 14:07, Keir Fraser <keir@xen.org> wrote:
> On 17/10/2012 12:53, "Jan Beulich" <JBeulich@suse.com> wrote:
>
>> Before anyone asks - yes, I have added to my todo list to put in
>> place a proper ioremap() for x86. But since I'm under some time
>> pressure right now, this was the faster route to something that
>> can be committed to see whether the regression goes away.
>
> That's good as we'll need it for >4kB per-VCPU stacks (even if we only need
> 4kB VCPU stacks, we'll want a guard page too).
But why would you want to do that via ioremap()?
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI/APEI: fix ERST MOVE_DATA instruction implementation
2012-10-17 12:15 ` Jan Beulich
@ 2012-10-17 13:01 ` Keir Fraser
0 siblings, 0 replies; 6+ messages in thread
From: Keir Fraser @ 2012-10-17 13:01 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On 17/10/2012 13:15, "Jan Beulich" <JBeulich@suse.com> wrote:
>>>> On 17.10.12 at 14:07, Keir Fraser <keir@xen.org> wrote:
>> On 17/10/2012 12:53, "Jan Beulich" <JBeulich@suse.com> wrote:
>>
>>> Before anyone asks - yes, I have added to my todo list to put in
>>> place a proper ioremap() for x86. But since I'm under some time
>>> pressure right now, this was the faster route to something that
>>> can be committed to see whether the regression goes away.
>>
>> That's good as we'll need it for >4kB per-VCPU stacks (even if we only need
>> 4kB VCPU stacks, we'll want a guard page too).
>
> But why would you want to do that via ioremap()?
Well the required missing mechanism is the same, right? The ability to
allocate some unused virtual address space. We already have
map_pages_to_xen(), so it's just the allocation part that is missing for
either ioremap() or vcpu stacks.
-- Keir
> Jan
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-10-17 13:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-17 11:36 [PATCH] ACPI/APEI: fix ERST MOVE_DATA instruction implementation Jan Beulich
2012-10-17 11:47 ` Keir Fraser
2012-10-17 11:53 ` Jan Beulich
2012-10-17 12:07 ` Keir Fraser
2012-10-17 12:15 ` Jan Beulich
2012-10-17 13:01 ` Keir Fraser
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.