target/arm/internals.h | 4 ++++ target/arm/mte_helper.c | 24 ++++++++++++++---------- target/arm/tlb_helper.c | 8 +++----- 3 files changed, 21 insertions(+), 15 deletions(-)
As reported by Andrey, I was missing the complete ISS info for the Data Abort raised upon a synchronous tag check fail. The following should fix that. All the twisty little rules for the ISS.ISV bit are already handled by merge_syn_data_abort. Probably the most important bit that was missing was ISS.WnR, as that is independent of ISS.ISV. Andrey, will you please test? r~ Richard Henderson (3): target/arm: Export merge_syn_data_abort from tlb_helper.c target/arm: Pass the entire mte descriptor to mte_check_fail target/arm: Merge ISS for data abort from tag check fail target/arm/internals.h | 4 ++++ target/arm/mte_helper.c | 24 ++++++++++++++---------- target/arm/tlb_helper.c | 8 +++----- 3 files changed, 21 insertions(+), 15 deletions(-) -- 2.25.1
On Wed, Aug 12, 2020 at 7:19 PM Richard Henderson <richard.henderson@linaro.org> wrote: > > As reported by Andrey, I was missing the complete ISS info for > the Data Abort raised upon a synchronous tag check fail. > > The following should fix that. All the twisty little rules for > the ISS.ISV bit are already handled by merge_syn_data_abort. > Probably the most important bit that was missing was ISS.WnR, > as that is independent of ISS.ISV. > > Andrey, will you please test? Looks like WnR is now being set properly, but SAS is still always 0. > > > r~ > > > Richard Henderson (3): > target/arm: Export merge_syn_data_abort from tlb_helper.c > target/arm: Pass the entire mte descriptor to mte_check_fail > target/arm: Merge ISS for data abort from tag check fail > > target/arm/internals.h | 4 ++++ > target/arm/mte_helper.c | 24 ++++++++++++++---------- > target/arm/tlb_helper.c | 8 +++----- > 3 files changed, 21 insertions(+), 15 deletions(-) > > -- > 2.25.1 >
On 8/12/20 10:38 AM, Andrey Konovalov wrote: > On Wed, Aug 12, 2020 at 7:19 PM Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> As reported by Andrey, I was missing the complete ISS info for >> the Data Abort raised upon a synchronous tag check fail. >> >> The following should fix that. All the twisty little rules for >> the ISS.ISV bit are already handled by merge_syn_data_abort. >> Probably the most important bit that was missing was ISS.WnR, >> as that is independent of ISS.ISV. >> >> Andrey, will you please test? > > Looks like WnR is now being set properly, but SAS is still always 0. Are you looking at ESR_EL1? On page D13-2992 of revision F.a: # ISV is 0 for all faults reported in ESR_EL1 or ESR_EL3. which means that ISS[23:14] are RES0, which includes SAS. r~
On 8/12/20 10:52 AM, Richard Henderson wrote: > On 8/12/20 10:38 AM, Andrey Konovalov wrote: >> On Wed, Aug 12, 2020 at 7:19 PM Richard Henderson >> <richard.henderson@linaro.org> wrote: >>> >>> As reported by Andrey, I was missing the complete ISS info for >>> the Data Abort raised upon a synchronous tag check fail. >>> >>> The following should fix that. All the twisty little rules for >>> the ISS.ISV bit are already handled by merge_syn_data_abort. >>> Probably the most important bit that was missing was ISS.WnR, >>> as that is independent of ISS.ISV. >>> >>> Andrey, will you please test? >> >> Looks like WnR is now being set properly, but SAS is still always 0. > > Are you looking at ESR_EL1? > > On page D13-2992 of revision F.a: > > # ISV is 0 for all faults reported in ESR_EL1 or ESR_EL3. > > which means that ISS[23:14] are RES0, which includes SAS. Actually, note that AArch64.TagCheckFault never fills in anything except WnR. So the final patch here merging the recorded syndrome information is wrong. I was only missing WnR in the original implementation. r~
On Wed, Aug 12, 2020 at 7:52 PM Richard Henderson <richard.henderson@linaro.org> wrote: > > On 8/12/20 10:38 AM, Andrey Konovalov wrote: > > On Wed, Aug 12, 2020 at 7:19 PM Richard Henderson > > <richard.henderson@linaro.org> wrote: > >> > >> As reported by Andrey, I was missing the complete ISS info for > >> the Data Abort raised upon a synchronous tag check fail. > >> > >> The following should fix that. All the twisty little rules for > >> the ISS.ISV bit are already handled by merge_syn_data_abort. > >> Probably the most important bit that was missing was ISS.WnR, > >> as that is independent of ISS.ISV. > >> > >> Andrey, will you please test? > > > > Looks like WnR is now being set properly, but SAS is still always 0. > > Are you looking at ESR_EL1? > > On page D13-2992 of revision F.a: > > # ISV is 0 for all faults reported in ESR_EL1 or ESR_EL3. > > which means that ISS[23:14] are RES0, which includes SAS. +more Arm and Google people Is this known? Do we not get access size when MTE fault happens?
On Wed, Aug 12, 2020 at 11:03 AM Andrey Konovalov <andreyknvl@google.com> wrote: > On Wed, Aug 12, 2020 at 7:52 PM Richard Henderson > <richard.henderson@linaro.org> wrote: > > > > On 8/12/20 10:38 AM, Andrey Konovalov wrote: > > > On Wed, Aug 12, 2020 at 7:19 PM Richard Henderson > > > <richard.henderson@linaro.org> wrote: > > >> > > >> As reported by Andrey, I was missing the complete ISS info for > > >> the Data Abort raised upon a synchronous tag check fail. > > >> > > >> The following should fix that. All the twisty little rules for > > >> the ISS.ISV bit are already handled by merge_syn_data_abort. > > >> Probably the most important bit that was missing was ISS.WnR, > > >> as that is independent of ISS.ISV. > > >> > > >> Andrey, will you please test? > > > > > > Looks like WnR is now being set properly, but SAS is still always 0. > > > > Are you looking at ESR_EL1? > > > > On page D13-2992 of revision F.a: > > > > # ISV is 0 for all faults reported in ESR_EL1 or ESR_EL3. > > > > which means that ISS[23:14] are RES0, which includes SAS. > > +more Arm and Google people > > Is this known? Do we not get access size when MTE fault happens? > It sounds like this applies to all data abort exceptions, no matter MTE or not.
On 12/08/2020 20:06, Evgenii Stepanov wrote: > On Wed, Aug 12, 2020 at 11:03 AM Andrey Konovalov <andreyknvl@google.com > <mailto:andreyknvl@google.com>> wrote: > > On Wed, Aug 12, 2020 at 7:52 PM Richard Henderson > <richard.henderson@linaro.org <mailto:richard.henderson@linaro.org>> wrote: > > > > On 8/12/20 10:38 AM, Andrey Konovalov wrote: > > > On Wed, Aug 12, 2020 at 7:19 PM Richard Henderson > > > <richard.henderson@linaro.org <mailto:richard.henderson@linaro.org>> wrote: > > >> > > >> As reported by Andrey, I was missing the complete ISS info for > > >> the Data Abort raised upon a synchronous tag check fail. > > >> > > >> The following should fix that. All the twisty little rules for > > >> the ISS.ISV bit are already handled by merge_syn_data_abort. > > >> Probably the most important bit that was missing was ISS.WnR, > > >> as that is independent of ISS.ISV. > > >> > > >> Andrey, will you please test? > > > > > > Looks like WnR is now being set properly, but SAS is still always 0. > > > > Are you looking at ESR_EL1? > > > > On page D13-2992 of revision F.a: > > > > # ISV is 0 for all faults reported in ESR_EL1 or ESR_EL3. > > > > which means that ISS[23:14] are RES0, which includes SAS. > > +more Arm and Google people > > Is this known? Do we not get access size when MTE fault happens? > > > It sounds like this applies to all data abort exceptions, no matter MTE or not. Correct. For data aborts in general, the extra syndrome information in ISS[23:14] is only provided at EL2, in order to help hypervisors emulate simple loads/stores (that access device memory) by looking at ESR_EL2 without having to decode the trapped instruction. Did you have any particular use-case in mind for SAS being set even in ESR_EL1? Kevin
On Thu, Aug 13, 2020 at 12:01 PM Kevin Brodsky <kevin.brodsky@arm.com> wrote: > > On 12/08/2020 20:06, Evgenii Stepanov wrote: > > On Wed, Aug 12, 2020 at 11:03 AM Andrey Konovalov <andreyknvl@google.com> wrote: >> >> On Wed, Aug 12, 2020 at 7:52 PM Richard Henderson >> <richard.henderson@linaro.org> wrote: >> > >> > On 8/12/20 10:38 AM, Andrey Konovalov wrote: >> > > On Wed, Aug 12, 2020 at 7:19 PM Richard Henderson >> > > <richard.henderson@linaro.org> wrote: >> > >> >> > >> As reported by Andrey, I was missing the complete ISS info for >> > >> the Data Abort raised upon a synchronous tag check fail. >> > >> >> > >> The following should fix that. All the twisty little rules for >> > >> the ISS.ISV bit are already handled by merge_syn_data_abort. >> > >> Probably the most important bit that was missing was ISS.WnR, >> > >> as that is independent of ISS.ISV. >> > >> >> > >> Andrey, will you please test? >> > > >> > > Looks like WnR is now being set properly, but SAS is still always 0. >> > >> > Are you looking at ESR_EL1? >> > >> > On page D13-2992 of revision F.a: >> > >> > # ISV is 0 for all faults reported in ESR_EL1 or ESR_EL3. >> > >> > which means that ISS[23:14] are RES0, which includes SAS. >> >> +more Arm and Google people >> >> Is this known? Do we not get access size when MTE fault happens? > > > It sounds like this applies to all data abort exceptions, no matter MTE or not. > > > Correct. For data aborts in general, the extra syndrome information in ISS[23:14] is only provided at EL2, in order to help hypervisors emulate simple loads/stores (that access device memory) by looking at ESR_EL2 without having to decode the trapped instruction. OK, got it. The WnR bit works properly though, so Tested-by: Andrey Konovalov <andreyknvl@google.com> for the series. > Did you have any particular use-case in mind for SAS being set even in ESR_EL1? Yes, we could use that to extract the size of the access that caused the fault to print more informative KASAN reports. We usually have a header like: BUG: KASAN: slab-out-of-bounds in usb_destroy_configuration+0x636/0x6d0 Read of size 8 at addr ffff8881c7e3c758 by task kworker/1:7/3434
© 2016 - 2025 Red Hat, Inc.