| Message ID | 20170811184404.17806-1-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
On 12 August 2017 at 00:14, Colin King <colin.king@canonical.com> wrote: > From: Colin Ian King <colin.king@canonical.com> > > Re-work fwts_acpi_get_rsdp so there are less memory copioes of the RSDP > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_acpi_tables.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_ > tables.c > index f7547c43..9b2312e5 100644 > --- a/src/lib/src/fwts_acpi_tables.c > +++ b/src/lib/src/fwts_acpi_tables.c > @@ -146,39 +146,37 @@ static void *fwts_acpi_find_rsdp_bios(void) > static fwts_acpi_table_rsdp *fwts_acpi_get_rsdp(fwts_framework *fw, void > *addr, size_t *rsdp_len) > { > uint8_t *mem; > - fwts_acpi_table_rsdp *rsdp = NULL; > + fwts_acpi_table_rsdp *rsdp; > *rsdp_len = 0; > > + /* Assume version 2.0 size, we don't care about a few bytes over > allocation if it's version 1.0 */ > + if ((rsdp = (fwts_acpi_table_rsdp*)fwts_low_calloc(1, > sizeof(fwts_acpi_table_rsdp))) == NULL) { > + fwts_log_error(fw, "Cannot allocate cached RSDP, out of > low memory."); > + return NULL; > + } > + > if ((mem = fwts_mmap((off_t)addr, sizeof(fwts_acpi_table_rsdp))) > == FWTS_MAP_FAILED) > return NULL; > > - if (fwts_safe_memread(mem, sizeof(fwts_acpi_table_rsdp)) != > FWTS_OK) { > - fwts_log_error(fw, "Cannot safely read RSDP from address > %p.", mem); > - goto out; > + if (fwts_safe_memcpy(rsdp, mem, sizeof(fwts_acpi_table_rsdp)) != > FWTS_OK) { > + fwts_log_error(fw, "Cannot safely copy RSDP from address > %p.", mem); > + goto err; > } > > - rsdp = (fwts_acpi_table_rsdp*)mem; > /* Determine original RSDP size from revision. */ > *rsdp_len = (rsdp->revision < 1) ? 20 : 36; > > /* Must have the correct signature */ > if (strncmp(rsdp->signature, "RSD PTR ", 8)) { > fwts_log_error(fw, "RSDP did not have expected > signature."); > - rsdp = NULL; > - goto out; > - } > - > - /* Assume version 2.0 size, we don't care about a few bytes over > allocation if it's version 1.0 */ > - if ((rsdp = (fwts_acpi_table_rsdp*)fwts_low_calloc(1, > sizeof(fwts_acpi_table_rsdp))) == NULL) { > - rsdp = NULL; > - goto out; > + goto err; > } > + return rsdp; > > - memcpy(rsdp, mem, *rsdp_len); > -out: > +err: > (void)fwts_munmap(mem, sizeof(fwts_acpi_table_rsdp)); > - > - return rsdp; > + fwts_low_free(rsdp); > + return NULL; > } > > /* > -- > 2.11.0 > Thanks Colin, Reviewed-By/Tested-By: Naresh Bhat <naresh.bhat@linaro.org> > > > -- > fwts-devel mailing list > fwts-devel@lists.ubuntu.com > Modify settings or unsubscribe at: https://lists.ubuntu.com/ > mailman/listinfo/fwts-devel >
On 8/11/2017 12:44 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Re-work fwts_acpi_get_rsdp so there are less memory copioes of the RSDP copioes -> copies
On 11/08/17 20:06, Jeffrey Hugo wrote: > On 8/11/2017 12:44 PM, Colin King wrote: >> From: Colin Ian King <colin.king@canonical.com> >> >> Re-work fwts_acpi_get_rsdp so there are less memory copioes of the RSDP > > copioes -> copies > Ivan/Alex, can you fix that typo up before applying the patch.
On 2017-08-11 11:44 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Re-work fwts_acpi_get_rsdp so there are less memory copioes of the RSDP > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_acpi_tables.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c > index f7547c43..9b2312e5 100644 > --- a/src/lib/src/fwts_acpi_tables.c > +++ b/src/lib/src/fwts_acpi_tables.c > @@ -146,39 +146,37 @@ static void *fwts_acpi_find_rsdp_bios(void) > static fwts_acpi_table_rsdp *fwts_acpi_get_rsdp(fwts_framework *fw, void *addr, size_t *rsdp_len) > { > uint8_t *mem; > - fwts_acpi_table_rsdp *rsdp = NULL; > + fwts_acpi_table_rsdp *rsdp; > *rsdp_len = 0; > > + /* Assume version 2.0 size, we don't care about a few bytes over allocation if it's version 1.0 */ > + if ((rsdp = (fwts_acpi_table_rsdp*)fwts_low_calloc(1, sizeof(fwts_acpi_table_rsdp))) == NULL) { > + fwts_log_error(fw, "Cannot allocate cached RSDP, out of low memory."); > + return NULL; > + } > + > if ((mem = fwts_mmap((off_t)addr, sizeof(fwts_acpi_table_rsdp))) == FWTS_MAP_FAILED) > return NULL; > > - if (fwts_safe_memread(mem, sizeof(fwts_acpi_table_rsdp)) != FWTS_OK) { > - fwts_log_error(fw, "Cannot safely read RSDP from address %p.", mem); > - goto out; > + if (fwts_safe_memcpy(rsdp, mem, sizeof(fwts_acpi_table_rsdp)) != FWTS_OK) { > + fwts_log_error(fw, "Cannot safely copy RSDP from address %p.", mem); > + goto err; > } > > - rsdp = (fwts_acpi_table_rsdp*)mem; > /* Determine original RSDP size from revision. */ > *rsdp_len = (rsdp->revision < 1) ? 20 : 36; > > /* Must have the correct signature */ > if (strncmp(rsdp->signature, "RSD PTR ", 8)) { > fwts_log_error(fw, "RSDP did not have expected signature."); > - rsdp = NULL; > - goto out; > - } > - > - /* Assume version 2.0 size, we don't care about a few bytes over allocation if it's version 1.0 */ > - if ((rsdp = (fwts_acpi_table_rsdp*)fwts_low_calloc(1, sizeof(fwts_acpi_table_rsdp))) == NULL) { > - rsdp = NULL; > - goto out; > + goto err; > } > + return rsdp; > > - memcpy(rsdp, mem, *rsdp_len); > -out: > +err: > (void)fwts_munmap(mem, sizeof(fwts_acpi_table_rsdp)); > - > - return rsdp; > + fwts_low_free(rsdp); > + return NULL; > } > > /* > Acked-by: Alex Hung <alex.hung@canonical.com>
On 08/12/2017 02:44 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Re-work fwts_acpi_get_rsdp so there are less memory copioes of the RSDP > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_acpi_tables.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c > index f7547c43..9b2312e5 100644 > --- a/src/lib/src/fwts_acpi_tables.c > +++ b/src/lib/src/fwts_acpi_tables.c > @@ -146,39 +146,37 @@ static void *fwts_acpi_find_rsdp_bios(void) > static fwts_acpi_table_rsdp *fwts_acpi_get_rsdp(fwts_framework *fw, void *addr, size_t *rsdp_len) > { > uint8_t *mem; > - fwts_acpi_table_rsdp *rsdp = NULL; > + fwts_acpi_table_rsdp *rsdp; > *rsdp_len = 0; > > + /* Assume version 2.0 size, we don't care about a few bytes over allocation if it's version 1.0 */ > + if ((rsdp = (fwts_acpi_table_rsdp*)fwts_low_calloc(1, sizeof(fwts_acpi_table_rsdp))) == NULL) { > + fwts_log_error(fw, "Cannot allocate cached RSDP, out of low memory."); > + return NULL; > + } > + > if ((mem = fwts_mmap((off_t)addr, sizeof(fwts_acpi_table_rsdp))) == FWTS_MAP_FAILED) > return NULL; > > - if (fwts_safe_memread(mem, sizeof(fwts_acpi_table_rsdp)) != FWTS_OK) { > - fwts_log_error(fw, "Cannot safely read RSDP from address %p.", mem); > - goto out; > + if (fwts_safe_memcpy(rsdp, mem, sizeof(fwts_acpi_table_rsdp)) != FWTS_OK) { > + fwts_log_error(fw, "Cannot safely copy RSDP from address %p.", mem); > + goto err; > } > > - rsdp = (fwts_acpi_table_rsdp*)mem; > /* Determine original RSDP size from revision. */ > *rsdp_len = (rsdp->revision < 1) ? 20 : 36; > > /* Must have the correct signature */ > if (strncmp(rsdp->signature, "RSD PTR ", 8)) { > fwts_log_error(fw, "RSDP did not have expected signature."); > - rsdp = NULL; > - goto out; > - } > - > - /* Assume version 2.0 size, we don't care about a few bytes over allocation if it's version 1.0 */ > - if ((rsdp = (fwts_acpi_table_rsdp*)fwts_low_calloc(1, sizeof(fwts_acpi_table_rsdp))) == NULL) { > - rsdp = NULL; > - goto out; > + goto err; > } > + return rsdp; > > - memcpy(rsdp, mem, *rsdp_len); > -out: > +err: > (void)fwts_munmap(mem, sizeof(fwts_acpi_table_rsdp)); > - > - return rsdp; > + fwts_low_free(rsdp); > + return NULL; > } > > /* > Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c index f7547c43..9b2312e5 100644 --- a/src/lib/src/fwts_acpi_tables.c +++ b/src/lib/src/fwts_acpi_tables.c @@ -146,39 +146,37 @@ static void *fwts_acpi_find_rsdp_bios(void) static fwts_acpi_table_rsdp *fwts_acpi_get_rsdp(fwts_framework *fw, void *addr, size_t *rsdp_len) { uint8_t *mem; - fwts_acpi_table_rsdp *rsdp = NULL; + fwts_acpi_table_rsdp *rsdp; *rsdp_len = 0; + /* Assume version 2.0 size, we don't care about a few bytes over allocation if it's version 1.0 */ + if ((rsdp = (fwts_acpi_table_rsdp*)fwts_low_calloc(1, sizeof(fwts_acpi_table_rsdp))) == NULL) { + fwts_log_error(fw, "Cannot allocate cached RSDP, out of low memory."); + return NULL; + } + if ((mem = fwts_mmap((off_t)addr, sizeof(fwts_acpi_table_rsdp))) == FWTS_MAP_FAILED) return NULL; - if (fwts_safe_memread(mem, sizeof(fwts_acpi_table_rsdp)) != FWTS_OK) { - fwts_log_error(fw, "Cannot safely read RSDP from address %p.", mem); - goto out; + if (fwts_safe_memcpy(rsdp, mem, sizeof(fwts_acpi_table_rsdp)) != FWTS_OK) { + fwts_log_error(fw, "Cannot safely copy RSDP from address %p.", mem); + goto err; } - rsdp = (fwts_acpi_table_rsdp*)mem; /* Determine original RSDP size from revision. */ *rsdp_len = (rsdp->revision < 1) ? 20 : 36; /* Must have the correct signature */ if (strncmp(rsdp->signature, "RSD PTR ", 8)) { fwts_log_error(fw, "RSDP did not have expected signature."); - rsdp = NULL; - goto out; - } - - /* Assume version 2.0 size, we don't care about a few bytes over allocation if it's version 1.0 */ - if ((rsdp = (fwts_acpi_table_rsdp*)fwts_low_calloc(1, sizeof(fwts_acpi_table_rsdp))) == NULL) { - rsdp = NULL; - goto out; + goto err; } + return rsdp; - memcpy(rsdp, mem, *rsdp_len); -out: +err: (void)fwts_munmap(mem, sizeof(fwts_acpi_table_rsdp)); - - return rsdp; + fwts_low_free(rsdp); + return NULL; } /*