| Message ID | 1588858868-18039-1-git-send-email-guohanjun@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ACPI: IORT: Add extra message "applying workaround" for off-by-1 issue | expand |
On Thu, 7 May 2020 at 15:47, Hanjun Guo <guohanjun@huawei.com> wrote: > > As we already applied a workaround for the off-by-1 issue, > it's good to add extra message "applying workaround" to > make people less uneasy to see such message in the boot log. > > Signed-off-by: Hanjun Guo <guohanjun@huawei.com> Hi Hanjun, > --- > > Based on top of for-next/acpi branch of ARM64 repo > > drivers/acpi/arm64/iort.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > index b011d25..f3d492a 100644 > --- a/drivers/acpi/arm64/iort.c > +++ b/drivers/acpi/arm64/iort.c > @@ -328,7 +328,7 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, > * Otherwise, things are *really* broken, and we just disregard > * duplicate matches entirely to retain compatibility. > */ > - pr_err(FW_BUG "[map %p] conflicting mapping for input ID 0x%x\n", > + pr_err(FW_BUG "[map %p] conflicting mapping for input ID 0x%x, applying workaround\n", This is not correct. The workaround is only applied if rid_in == map->input_base, so better to print a second line after the 'return' below that is only reached in that particular case. > map, rid_in); > if (rid_in != map->input_base) > return -ENXIO; > -- > 1.7.12.4 >
Hi Ard, On 2020/5/7 21:49, Ard Biesheuvel wrote: > On Thu, 7 May 2020 at 15:47, Hanjun Guo <guohanjun@huawei.com> wrote: >> >> As we already applied a workaround for the off-by-1 issue, >> it's good to add extra message "applying workaround" to >> make people less uneasy to see such message in the boot log. >> >> Signed-off-by: Hanjun Guo <guohanjun@huawei.com> > > Hi Hanjun, > >> --- >> >> Based on top of for-next/acpi branch of ARM64 repo >> >> drivers/acpi/arm64/iort.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >> index b011d25..f3d492a 100644 >> --- a/drivers/acpi/arm64/iort.c >> +++ b/drivers/acpi/arm64/iort.c >> @@ -328,7 +328,7 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, >> * Otherwise, things are *really* broken, and we just disregard >> * duplicate matches entirely to retain compatibility. >> */ >> - pr_err(FW_BUG "[map %p] conflicting mapping for input ID 0x%x\n", >> + pr_err(FW_BUG "[map %p] conflicting mapping for input ID 0x%x, applying workaround\n", > > This is not correct. The workaround is only applied if rid_in == > map->input_base, so better to print a second line after the 'return' > below that is only reached in that particular case. Obvious I'm wrong, I will update this patch, thanks a lot! > >> map, rid_in); >> if (rid_in != map->input_base) >> return -ENXIO; >> -- >> 1.7.12.4 >> > > . >
On 2020/5/7 21:53, Hanjun Guo wrote: > Hi Ard, > > On 2020/5/7 21:49, Ard Biesheuvel wrote: >> On Thu, 7 May 2020 at 15:47, Hanjun Guo <guohanjun@huawei.com> wrote: >>> >>> As we already applied a workaround for the off-by-1 issue, >>> it's good to add extra message "applying workaround" to >>> make people less uneasy to see such message in the boot log. >>> >>> Signed-off-by: Hanjun Guo <guohanjun@huawei.com> >> >> Hi Hanjun, >> >>> --- >>> >>> Based on top of for-next/acpi branch of ARM64 repo >>> >>> drivers/acpi/arm64/iort.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >>> index b011d25..f3d492a 100644 >>> --- a/drivers/acpi/arm64/iort.c >>> +++ b/drivers/acpi/arm64/iort.c >>> @@ -328,7 +328,7 @@ static int iort_id_map(struct >>> acpi_iort_id_mapping *map, u8 type, u32 rid_in, >>> * Otherwise, things are *really* broken, and we >>> just disregard >>> * duplicate matches entirely to retain compatibility. >>> */ >>> - pr_err(FW_BUG "[map %p] conflicting mapping for input >>> ID 0x%x\n", >>> + pr_err(FW_BUG "[map %p] conflicting mapping for input >>> ID 0x%x, applying workaround\n", >> >> This is not correct. The workaround is only applied if rid_in == >> map->input_base, so better to print a second line after the 'return' >> below that is only reached in that particular case. > > Obvious I'm wrong, I will update this patch, thanks a lot! By the way, how about the print below? Should I add something more? if (rid_in != map->input_base) return -ENXIO; + + pr_err(FW_BUG "applying workaround for the conflicting mapping\n"); Thanks Hanjun
On Thu, 7 May 2020 at 16:02, Hanjun Guo <guohanjun@huawei.com> wrote: > > On 2020/5/7 21:53, Hanjun Guo wrote: > > Hi Ard, > > > > On 2020/5/7 21:49, Ard Biesheuvel wrote: > >> On Thu, 7 May 2020 at 15:47, Hanjun Guo <guohanjun@huawei.com> wrote: > >>> > >>> As we already applied a workaround for the off-by-1 issue, > >>> it's good to add extra message "applying workaround" to > >>> make people less uneasy to see such message in the boot log. > >>> > >>> Signed-off-by: Hanjun Guo <guohanjun@huawei.com> > >> > >> Hi Hanjun, > >> > >>> --- > >>> > >>> Based on top of for-next/acpi branch of ARM64 repo > >>> > >>> drivers/acpi/arm64/iort.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > >>> index b011d25..f3d492a 100644 > >>> --- a/drivers/acpi/arm64/iort.c > >>> +++ b/drivers/acpi/arm64/iort.c > >>> @@ -328,7 +328,7 @@ static int iort_id_map(struct > >>> acpi_iort_id_mapping *map, u8 type, u32 rid_in, > >>> * Otherwise, things are *really* broken, and we > >>> just disregard > >>> * duplicate matches entirely to retain compatibility. > >>> */ > >>> - pr_err(FW_BUG "[map %p] conflicting mapping for input > >>> ID 0x%x\n", > >>> + pr_err(FW_BUG "[map %p] conflicting mapping for input > >>> ID 0x%x, applying workaround\n", > >> > >> This is not correct. The workaround is only applied if rid_in == > >> map->input_base, so better to print a second line after the 'return' > >> below that is only reached in that particular case. > > > > Obvious I'm wrong, I will update this patch, thanks a lot! > > By the way, how about the print below? Should I add something more? > > if (rid_in != map->input_base) > return -ENXIO; > + > + pr_err(FW_BUG "applying workaround for the conflicting > mapping\n"); > Simply 'applying workaround' should do it imo.
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index b011d25..f3d492a 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -328,7 +328,7 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, * Otherwise, things are *really* broken, and we just disregard * duplicate matches entirely to retain compatibility. */ - pr_err(FW_BUG "[map %p] conflicting mapping for input ID 0x%x\n", + pr_err(FW_BUG "[map %p] conflicting mapping for input ID 0x%x, applying workaround\n", map, rid_in); if (rid_in != map->input_base) return -ENXIO;
As we already applied a workaround for the off-by-1 issue, it's good to add extra message "applying workaround" to make people less uneasy to see such message in the boot log. Signed-off-by: Hanjun Guo <guohanjun@huawei.com> --- Based on top of for-next/acpi branch of ARM64 repo drivers/acpi/arm64/iort.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)