| Message ID | 20170526061829.9261-1-oohall@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, May 26, 2017 at 04:18:29PM +1000, Oliver O'Halloran wrote: >Hostboot should name any and all memory reservations that it provides. >Currently some hostboots export a broken reservation covering the first >256MB of memory and this causes the system to crash at boot due to an >invalid free because this overlaps with the static "ibm,os-reserve" >region (which covers the first 768MB of memory). > >According to the hostboot team unnamed reservations are invalid and can >be ignored. > >Signed-off-by: Oliver O'Halloran <oohall@gmail.com> Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com> Tested-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >--- > hdata/memory.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > >diff --git a/hdata/memory.c b/hdata/memory.c >index 0c6c0811f61a..26c8a9837b73 100644 >--- a/hdata/memory.c >+++ b/hdata/memory.c >@@ -409,7 +409,7 @@ static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd) > { > const struct msvpd_hb_reserved_mem *hb_resv_mem; > u64 start_addr, end_addr, label_size; >- int unnamed = 0, count, i; >+ int count, i; > char *label; > > /* >@@ -463,9 +463,9 @@ static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd) > memcpy(label, hb_resv_mem->label, label_size); > label[label_size] = '\0'; > >+ /* Unnamed reservations are always broken. Ignore them. */ > if (strlen(label) == 0) >- snprintf(label, 64, "hostboot-reserve-%d", unnamed++); >- >+ continue; When the memory block allocated and tracked by @label isn't used, it needs to be free'd? However, I believe not too much memory wasted though :) > > prlog(PR_DEBUG, "MEM: Reserve '%s' %#" PRIx64 "-%#" PRIx64 " (type/inst=0x%08x)\n", > label, start_addr, end_addr, be32_to_cpu(hb_resv_mem->type_instance)); Cheers, Gavin
On Fri, May 26, 2017 at 4:39 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote: > On Fri, May 26, 2017 at 04:18:29PM +1000, Oliver O'Halloran wrote: >>Hostboot should name any and all memory reservations that it provides. >>Currently some hostboots export a broken reservation covering the first >>256MB of memory and this causes the system to crash at boot due to an >>invalid free because this overlaps with the static "ibm,os-reserve" >>region (which covers the first 768MB of memory). >> >>According to the hostboot team unnamed reservations are invalid and can >>be ignored. >> >>Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > > Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > Tested-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > >>--- >> hdata/memory.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >>diff --git a/hdata/memory.c b/hdata/memory.c >>index 0c6c0811f61a..26c8a9837b73 100644 >>--- a/hdata/memory.c >>+++ b/hdata/memory.c >>@@ -409,7 +409,7 @@ static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd) >> { >> const struct msvpd_hb_reserved_mem *hb_resv_mem; >> u64 start_addr, end_addr, label_size; >>- int unnamed = 0, count, i; >>+ int count, i; >> char *label; >> >> /* >>@@ -463,9 +463,9 @@ static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd) >> memcpy(label, hb_resv_mem->label, label_size); >> label[label_size] = '\0'; >> >>+ /* Unnamed reservations are always broken. Ignore them. */ >> if (strlen(label) == 0) >>- snprintf(label, 64, "hostboot-reserve-%d", unnamed++); >>- >>+ continue; > > When the memory block allocated and tracked by @label isn't used, it needs to be > free'd? However, I believe not too much memory wasted though :) Ah yeah, true. That used to be stack-allocated. > >> >> prlog(PR_DEBUG, "MEM: Reserve '%s' %#" PRIx64 "-%#" PRIx64 " (type/inst=0x%08x)\n", >> label, start_addr, end_addr, be32_to_cpu(hb_resv_mem->type_instance)); > > Cheers, > Gavin >
diff --git a/hdata/memory.c b/hdata/memory.c index 0c6c0811f61a..26c8a9837b73 100644 --- a/hdata/memory.c +++ b/hdata/memory.c @@ -409,7 +409,7 @@ static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd) { const struct msvpd_hb_reserved_mem *hb_resv_mem; u64 start_addr, end_addr, label_size; - int unnamed = 0, count, i; + int count, i; char *label; /* @@ -463,9 +463,9 @@ static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd) memcpy(label, hb_resv_mem->label, label_size); label[label_size] = '\0'; + /* Unnamed reservations are always broken. Ignore them. */ if (strlen(label) == 0) - snprintf(label, 64, "hostboot-reserve-%d", unnamed++); - + continue; prlog(PR_DEBUG, "MEM: Reserve '%s' %#" PRIx64 "-%#" PRIx64 " (type/inst=0x%08x)\n", label, start_addr, end_addr, be32_to_cpu(hb_resv_mem->type_instance));
Hostboot should name any and all memory reservations that it provides. Currently some hostboots export a broken reservation covering the first 256MB of memory and this causes the system to crash at boot due to an invalid free because this overlaps with the static "ibm,os-reserve" region (which covers the first 768MB of memory). According to the hostboot team unnamed reservations are invalid and can be ignored. Signed-off-by: Oliver O'Halloran <oohall@gmail.com> --- hdata/memory.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)