| Message ID | 1384135666-1929-1-git-send-email-gong.chen@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Sun, Nov 10, 2013 at 09:07:46PM -0500, Chen, Gong wrote: > Date: Sun, 10 Nov 2013 21:07:46 -0500 > From: "Chen, Gong" <gong.chen@linux.intel.com> > To: tony.luck@intel.com, bp@alien8.de > Cc: linux-acpi@vger.kernel.org, "Chen, Gong" <gong.chen@linux.intel.com> > Subject: [PATCH V2] ACPI, APEI: Cleanup alignment related codes for APEI > X-Mailer: git-send-email 1.8.4.rc3 > > We ever used *memcpy* to avoid access alignment issue between > firmware and OS. Now we can use a better and standard way > to avoid this issue. In the meanwhile, simplify some variable names > to avoid the limit of 80 characters per line and use structure > assignment instead of unnecessary memcpy. No functional changes. > > v2->v1: Make description information clearer. > Any comments? Boris/Tony?
On Wed, Nov 13, 2013 at 08:49:24PM -0500, Chen, Gong wrote: > On Sun, Nov 10, 2013 at 09:07:46PM -0500, Chen, Gong wrote: > > Date: Sun, 10 Nov 2013 21:07:46 -0500 > > From: "Chen, Gong" <gong.chen@linux.intel.com> > > To: tony.luck@intel.com, bp@alien8.de > > Cc: linux-acpi@vger.kernel.org, "Chen, Gong" <gong.chen@linux.intel.com> > > Subject: [PATCH V2] ACPI, APEI: Cleanup alignment related codes for APEI > > X-Mailer: git-send-email 1.8.4.rc3 > > > > We ever used *memcpy* to avoid access alignment issue between > > firmware and OS. Now we can use a better and standard way > > to avoid this issue. In the meanwhile, simplify some variable names > > to avoid the limit of 80 characters per line and use structure > > assignment instead of unnecessary memcpy. No functional changes. > > > > v2->v1: Make description information clearer. > > > Any comments? Boris/Tony? I get this when building here: drivers/acpi/apei/apei-base.c: In function ‘apei_check_gar’: drivers/acpi/apei/apei-base.c:571:8: warning: assignment makes pointer from integer without a cast [enabled by default] paddr = get_unaligned(®->address); ^ drivers/acpi/apei/apei-base.c: In function ‘collect_res_callback’: drivers/acpi/apei/apei-base.c:716:3: warning: ‘paddr’ may be used uninitialized in this function [-Wmaybe-uninitialized] return apei_res_add(&resources->iomem, paddr, ^ drivers/acpi/apei/apei-base.c: In function ‘apei_read’: drivers/acpi/apei/apei-base.c:645:10: warning: ‘address’ may be used uninitialized in this function [-Wmaybe-uninitialized] status = acpi_os_read_memory((acpi_physical_address) address, ^ drivers/acpi/apei/apei-base.c: In function ‘apei_write’: drivers/acpi/apei/apei-base.c:678:10: warning: ‘address’ may be used uninitialized in this function [-Wmaybe-uninitialized] status = acpi_os_write_memory((acpi_physical_address) address,
On Thu, Nov 14, 2013 at 01:29:56PM +0100, Borislav Petkov wrote: > Date: Thu, 14 Nov 2013 13:29:56 +0100 > From: Borislav Petkov <bp@alien8.de> > To: "Chen, Gong" <gong.chen@linux.intel.com> > Cc: tony.luck@intel.com, linux-acpi@vger.kernel.org > Subject: Re: [PATCH V2] ACPI, APEI: Cleanup alignment related codes for APEI > User-Agent: Mutt/1.5.21 (2010-09-15) > > On Wed, Nov 13, 2013 at 08:49:24PM -0500, Chen, Gong wrote: > > On Sun, Nov 10, 2013 at 09:07:46PM -0500, Chen, Gong wrote: > > > Date: Sun, 10 Nov 2013 21:07:46 -0500 > > > From: "Chen, Gong" <gong.chen@linux.intel.com> > > > To: tony.luck@intel.com, bp@alien8.de > > > Cc: linux-acpi@vger.kernel.org, "Chen, Gong" <gong.chen@linux.intel.com> > > > Subject: [PATCH V2] ACPI, APEI: Cleanup alignment related codes for APEI > > > X-Mailer: git-send-email 1.8.4.rc3 > > > > > > We ever used *memcpy* to avoid access alignment issue between > > > firmware and OS. Now we can use a better and standard way > > > to avoid this issue. In the meanwhile, simplify some variable names > > > to avoid the limit of 80 characters per line and use structure > > > assignment instead of unnecessary memcpy. No functional changes. > > > > > > v2->v1: Make description information clearer. > > > > > Any comments? Boris/Tony? > > I get this when building here: > > drivers/acpi/apei/apei-base.c: In function ‘apei_check_gar’: > drivers/acpi/apei/apei-base.c:571:8: warning: assignment makes pointer from integer without a cast [enabled by default] > paddr = get_unaligned(®->address); > ^ Gee, it is really really a stupid error. I thought I checked the patch throughly but I'm wrong :-(. It should be *paddr = get_unaligned(®->address); > drivers/acpi/apei/apei-base.c: In function ‘collect_res_callback’: > drivers/acpi/apei/apei-base.c:716:3: warning: ‘paddr’ may be used uninitialized in this function [-Wmaybe-uninitialized] > return apei_res_add(&resources->iomem, paddr, > ^ > drivers/acpi/apei/apei-base.c: In function ‘apei_read’: > drivers/acpi/apei/apei-base.c:645:10: warning: ‘address’ may be used uninitialized in this function [-Wmaybe-uninitialized] > status = acpi_os_read_memory((acpi_physical_address) address, > ^ > drivers/acpi/apei/apei-base.c: In function ‘apei_write’: > drivers/acpi/apei/apei-base.c:678:10: warning: ‘address’ may be used uninitialized in this function [-Wmaybe-uninitialized] > status = acpi_os_write_memory((acpi_physical_address) address, > > -- > Regards/Gruss, > Boris. > > Sent from a fat crate under my desk. Formatting is fine. > --
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c index 46f80e2..c481adf 100644 --- a/drivers/acpi/apei/apei-base.c +++ b/drivers/acpi/apei/apei-base.c @@ -41,6 +41,7 @@ #include <linux/rculist.h> #include <linux/interrupt.h> #include <linux/debugfs.h> +#include <asm/unaligned.h> #include "apei-internal.h" @@ -567,8 +568,7 @@ static int apei_check_gar(struct acpi_generic_address *reg, u64 *paddr, bit_offset = reg->bit_offset; access_size_code = reg->access_width; space_id = reg->space_id; - /* Handle possible alignment issues */ - memcpy(paddr, ®->address, sizeof(*paddr)); + paddr = get_unaligned(®->address); if (!*paddr) { pr_warning(FW_BUG APEI_PFX "Invalid physical address in GAR [0x%llx/%u/%u/%u/%u]\n", diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c index fb57d03..361177a 100644 --- a/drivers/acpi/apei/einj.c +++ b/drivers/acpi/apei/einj.c @@ -34,6 +34,7 @@ #include <linux/delay.h> #include <linux/mm.h> #include <acpi/acpi.h> +#include <asm/unaligned.h> #include "apei-internal.h" @@ -216,7 +217,7 @@ static void check_vendor_extension(u64 paddr, static void *einj_get_parameter_address(void) { int i; - u64 paddrv4 = 0, paddrv5 = 0; + u64 pa_v4 = 0, pa_v5 = 0; struct acpi_whea_header *entry; entry = EINJ_TAB_ENTRY(einj_tab); @@ -225,30 +226,28 @@ static void *einj_get_parameter_address(void) entry->instruction == ACPI_EINJ_WRITE_REGISTER && entry->register_region.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) - memcpy(&paddrv4, &entry->register_region.address, - sizeof(paddrv4)); + pa_v4 = get_unaligned(&entry->register_region.address); if (entry->action == ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS && entry->instruction == ACPI_EINJ_WRITE_REGISTER && entry->register_region.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) - memcpy(&paddrv5, &entry->register_region.address, - sizeof(paddrv5)); + pa_v5 = get_unaligned(&entry->register_region.address); entry++; } - if (paddrv5) { + if (pa_v5) { struct set_error_type_with_address *v5param; - v5param = acpi_os_map_memory(paddrv5, sizeof(*v5param)); + v5param = acpi_os_map_memory(pa_v5, sizeof(*v5param)); if (v5param) { acpi5 = 1; - check_vendor_extension(paddrv5, v5param); + check_vendor_extension(pa_v5, v5param); return v5param; } } - if (param_extension && paddrv4) { + if (param_extension && pa_v4) { struct einj_parameter *v4param; - v4param = acpi_os_map_memory(paddrv4, sizeof(*v4param)); + v4param = acpi_os_map_memory(pa_v4, sizeof(*v4param)); if (!v4param) return NULL; if (v4param->reserved1 || v4param->reserved2) { diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index 26311f2..bf30a12 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -611,7 +611,7 @@ static void __erst_record_id_cache_compact(void) if (entries[i] == APEI_ERST_INVALID_RECORD_ID) continue; if (wpos != i) - memcpy(&entries[wpos], &entries[i], sizeof(entries[i])); + entries[wpos] = entries[i]; wpos++; } erst_record_id_cache.len = wpos;
We ever used *memcpy* to avoid access alignment issue between firmware and OS. Now we can use a better and standard way to avoid this issue. In the meanwhile, simplify some variable names to avoid the limit of 80 characters per line and use structure assignment instead of unnecessary memcpy. No functional changes. v2->v1: Make description information clearer. Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> --- drivers/acpi/apei/apei-base.c | 4 ++-- drivers/acpi/apei/einj.c | 19 +++++++++---------- drivers/acpi/apei/erst.c | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-)