From: James Morse <james.morse@arm.com> To: Mark Rutland <mark.rutland@arm.com>, Colin King <colin.king@canonical.com> Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>, Len Brown <lenb@kernel.org>, Alexander Viro <viro@zeniv.linux.org.uk>, Andrew Morton <akpm@linux-foundation.org>, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, lorenzo.pieralisi@arm.com Subject: Re: [PATCH][V2] ACPI: sysfs: copy ACPI data using io memory copying Date: Wed, 1 Apr 2020 13:44:34 +0100 [thread overview] Message-ID: <698da6fc-3334-5420-5c97-4406914e4599@arm.com> (raw) In-Reply-To: <20200320131951.GA6555@lakrids.cambridge.arm.com> Hello! On 3/20/20 1:19 PM, Mark Rutland wrote: > [adding James and Lorenzo] (but not actually...) > On Tue, Mar 17, 2020 at 04:54:09PM +0000, Colin King wrote: >> From: Colin Ian King <colin.king@canonical.com> >> >> Reading ACPI data on ARM64 at a non-aligned offset from >> /sys/firmware/acpi/tables/data/BERT will cause a splat because >> the data is I/O memory mapped On your platform, on someone else's it may be in memory. Which platform is this on? (I've never seen one generate a BERT!) >> and being read with just a memcpy. >> Fix this by introducing an I/O variant of memory_read_from_buffer >> and using I/O memory mapped copies instead. > Just to check, is that correct is it correct to map those tables with > Device attributes in the first place, or should we be mapping the tables > with Normal Cacheable attributes with memremap()? > > If the FW placed those into memory using cacheavble attributes, reading > them using Device attributes could result in stale values, which could > be garbage. Yes. The BERT code should be using arch_apei_get_mem_attribute() to use the correct attributes. See ghes_map() for an example. bert_init() will need to use a version of ioremap() that takes the pgprot_t. Always using ioremap_cache() means you get a cacheable mapping, regardless of how firmware described this region in the UEFI memory map. This doesn't explain why you got an alignment fault. Otherwise, looks fine to me. (N.B. I ignored this patch as it wasn't copied to linux-arm-kernel and the subject says its about sysfs<->ACPI, nothing to do with APEI!) Thanks, James
WARNING: multiple messages have this Message-ID (diff)
From: James Morse <james.morse@arm.com> To: Mark Rutland <mark.rutland@arm.com>, Colin King <colin.king@canonical.com> Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>, Len Brown <lenb@kernel.org>, Alexander Viro <viro@zeniv.linux.org.uk>, Andrew Morton <akpm@linux-foundation.org>, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, lorenzo.pieralisi@arm.com Subject: Re: [PATCH][V2] ACPI: sysfs: copy ACPI data using io memory copying Date: Wed, 01 Apr 2020 12:44:34 +0000 [thread overview] Message-ID: <698da6fc-3334-5420-5c97-4406914e4599@arm.com> (raw) In-Reply-To: <20200320131951.GA6555@lakrids.cambridge.arm.com> Hello! On 3/20/20 1:19 PM, Mark Rutland wrote: > [adding James and Lorenzo] (but not actually...) > On Tue, Mar 17, 2020 at 04:54:09PM +0000, Colin King wrote: >> From: Colin Ian King <colin.king@canonical.com> >> >> Reading ACPI data on ARM64 at a non-aligned offset from >> /sys/firmware/acpi/tables/data/BERT will cause a splat because >> the data is I/O memory mapped On your platform, on someone else's it may be in memory. Which platform is this on? (I've never seen one generate a BERT!) >> and being read with just a memcpy. >> Fix this by introducing an I/O variant of memory_read_from_buffer >> and using I/O memory mapped copies instead. > Just to check, is that correct is it correct to map those tables with > Device attributes in the first place, or should we be mapping the tables > with Normal Cacheable attributes with memremap()? > > If the FW placed those into memory using cacheavble attributes, reading > them using Device attributes could result in stale values, which could > be garbage. Yes. The BERT code should be using arch_apei_get_mem_attribute() to use the correct attributes. See ghes_map() for an example. bert_init() will need to use a version of ioremap() that takes the pgprot_t. Always using ioremap_cache() means you get a cacheable mapping, regardless of how firmware described this region in the UEFI memory map. This doesn't explain why you got an alignment fault. Otherwise, looks fine to me. (N.B. I ignored this patch as it wasn't copied to linux-arm-kernel and the subject says its about sysfs<->ACPI, nothing to do with APEI!) Thanks, James
next prev parent reply other threads:[~2020-04-01 12:44 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-17 16:54 [PATCH][V2] ACPI: sysfs: copy ACPI data using io memory copying Colin King 2020-03-17 16:54 ` Colin King 2020-03-17 17:50 ` Rafael J. Wysocki 2020-03-17 17:50 ` Rafael J. Wysocki 2020-03-20 13:19 ` Mark Rutland 2020-03-20 13:19 ` Mark Rutland 2020-04-01 12:44 ` James Morse [this message] 2020-04-01 12:44 ` James Morse 2022-02-28 23:51 ` Henry Willard 2022-03-01 16:00 ` Lorenzo Pieralisi 2022-03-07 21:22 ` doug rady OS 2022-04-01 18:03 ` doug rady OS
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=698da6fc-3334-5420-5c97-4406914e4599@arm.com \ --to=james.morse@arm.com \ --cc=akpm@linux-foundation.org \ --cc=colin.king@canonical.com \ --cc=kernel-janitors@vger.kernel.org \ --cc=lenb@kernel.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=mark.rutland@arm.com \ --cc=rjw@rjwysocki.net \ --cc=viro@zeniv.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.