diff mbox series

[v5,15/22] target/arm: Clean address for DC ZVA

Message ID 20191011134744.2477-16-richard.henderson@linaro.org
State Superseded
Headers show
Series [v5,01/22] target/arm: Add MTE_ACTIVE to tb_flags | expand

Commit Message

Richard Henderson Oct. 11, 2019, 1:47 p.m. UTC
This data access was forgotten in the previous patch.

Fixes: 3a471103ac1823ba
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/arm/translate-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.17.1

Comments

Peter Maydell Dec. 5, 2019, 5:54 p.m. UTC | #1
On Fri, 11 Oct 2019 at 14:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> This data access was forgotten in the previous patch.


Do you mean "in the patch where we added support for cleaning
addresses of TBI information"? As written it sounds like you're
referring to the previous patch in this patchseries.

> Fixes: 3a471103ac1823ba

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>


thanks
-- PMM
Peter Maydell Dec. 5, 2019, 6:58 p.m. UTC | #2
On Fri, 11 Oct 2019 at 14:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> This data access was forgotten in the previous patch.

>

> Fixes: 3a471103ac1823ba

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/arm/translate-a64.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c

> index 4e049bb4aa..49817b96ae 100644

> --- a/target/arm/translate-a64.c

> +++ b/target/arm/translate-a64.c

> @@ -1766,7 +1766,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread,

>          return;

>      case ARM_CP_DC_ZVA:

>          /* Writes clear the aligned block of memory which rt points into. */

> -        tcg_rt = cpu_reg(s, rt);

> +        tcg_rt = clean_data_tbi(s, cpu_reg(s, rt), false);

>          gen_helper_dc_zva(cpu_env, tcg_rt);

>          return;


...doesn't this mean we don't do a tag check for DC ZVA?
Or is that handled in the helper ? (I guess it has to be,
the DC ZVA will span multiple tag granules).

thanks
-- PMM
Richard Henderson Feb. 18, 2020, 12:50 a.m. UTC | #3
On 12/5/19 10:58 AM, Peter Maydell wrote:
> On Fri, 11 Oct 2019 at 14:50, Richard Henderson

> <richard.henderson@linaro.org> wrote:

>>

>> This data access was forgotten in the previous patch.

>>

>> Fixes: 3a471103ac1823ba

>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>> ---

>>  target/arm/translate-a64.c | 2 +-

>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>

>> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c

>> index 4e049bb4aa..49817b96ae 100644

>> --- a/target/arm/translate-a64.c

>> +++ b/target/arm/translate-a64.c

>> @@ -1766,7 +1766,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread,

>>          return;

>>      case ARM_CP_DC_ZVA:

>>          /* Writes clear the aligned block of memory which rt points into. */

>> -        tcg_rt = cpu_reg(s, rt);

>> +        tcg_rt = clean_data_tbi(s, cpu_reg(s, rt), false);

>>          gen_helper_dc_zva(cpu_env, tcg_rt);

>>          return;

> 

> ...doesn't this mean we don't do a tag check for DC ZVA?

> Or is that handled in the helper ? (I guess it has to be,

> the DC ZVA will span multiple tag granules).


It isn't clear to me that DC ZVA does check tags.  There's no pseudocode for
DC_ZVA, which is less helpful than it might be.

I suppose one could infer it from "behaves as a set of stores to each byte
within the block", but considering that stores are both checking or
non-checking depending on addressing mode etc, I'd have preferred an explicit note.

So far I'm treating DC_ZVA as unchecked.


r~
Peter Maydell Feb. 18, 2020, 11:10 a.m. UTC | #4
On Tue, 18 Feb 2020 at 00:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
> It isn't clear to me that DC ZVA does check tags.  There's no pseudocode for

> DC_ZVA, which is less helpful than it might be.

>

> I suppose one could infer it from "behaves as a set of stores to each byte

> within the block", but considering that stores are both checking or

> non-checking depending on addressing mode etc, I'd have preferred an explicit note.

>

> So far I'm treating DC_ZVA as unchecked.


I think the spec is reasonably clear here. In D6.8 it says
"Unless an access is explicitly defined as a Tag Unchecked
access it is a Tag Checked access."
and then in D6.8.1:
"A cache maintenance by virtual address operation other than
DC ZVA generates a Tag Unchecked access."

So DC ZVA is Tag Checked, other DC ops that take a VA
are Tag Unchecked.

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 4e049bb4aa..49817b96ae 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1766,7 +1766,7 @@  static void handle_sys(DisasContext *s, uint32_t insn, bool isread,
         return;
     case ARM_CP_DC_ZVA:
         /* Writes clear the aligned block of memory which rt points into. */
-        tcg_rt = cpu_reg(s, rt);
+        tcg_rt = clean_data_tbi(s, cpu_reg(s, rt), false);
         gen_helper_dc_zva(cpu_env, tcg_rt);
         return;
     default: