| Message ID | 20230810023548.412310-5-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Implement cortex-a710 | expand |
On Thu, 10 Aug 2023 at 03:36, Richard Henderson <richard.henderson@linaro.org> wrote: > > Support all of the easy GM block sizes. > Use direct memory operations, since the pointers are aligned. > > While BS=2 (16 bytes, 1 tag) is a legal setting, that requires > an atomic store of one nibble. This is not difficult, but there > is also no point in supporting it until required. > > Note that cortex-a710 sets GM blocksize to match its cacheline > size of 64 bytes. I expect many implementations will also > match the cacheline, which makes 16 bytes very unlikely. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/tcg/mte_helper.c | 61 ++++++++++++++++++++++++++++++++----- > 1 file changed, 53 insertions(+), 8 deletions(-) > > diff --git a/target/arm/tcg/mte_helper.c b/target/arm/tcg/mte_helper.c > index 3640c6e57f..6faf4e42d5 100644 > --- a/target/arm/tcg/mte_helper.c > +++ b/target/arm/tcg/mte_helper.c > @@ -428,6 +428,8 @@ uint64_t HELPER(ldgm)(CPUARMState *env, uint64_t ptr) > int gm_bs = env_archcpu(env)->gm_blocksize; > int gm_bs_bytes = 4 << gm_bs; > void *tag_mem; > + uint64_t ret; > + int shift; > > ptr = QEMU_ALIGN_DOWN(ptr, gm_bs_bytes); > > @@ -443,16 +445,39 @@ uint64_t HELPER(ldgm)(CPUARMState *env, uint64_t ptr) > > /* > * The ordering of elements within the word corresponds to > - * a little-endian operation. > + * a little-endian operation. Computation of shift comes from > + * > + * index = address<LOG2_TAG_GRANULE+3:LOG2_TAG_GRANULE> > + * data<index*4+3:index*4> = tag > + * > + * Because of the alignment of ptr above, BS=6 has shift=0. > + * All memory operations are aligned. > */ > switch (gm_bs) { > - case 6: > - /* 256 bytes -> 16 tags -> 64 result bits */ > - return ldq_le_p(tag_mem); > - default: > + case 2: > /* cpu configured with unsupported gm blocksize. */ > g_assert_not_reached(); > + case 3: > + /* 32 bytes -> 2 tags -> 8 result bits */ > + ret = *(uint8_t *)tag_mem; > + break; > + case 4: > + /* 64 bytes -> 4 tags -> 16 result bits */ > + ret = cpu_to_le16(*(uint16_t *)tag_mem); Does this really make a difference compared to ldw_le_p() ? > + break; > + case 5: > + /* 128 bytes -> 8 tags -> 32 result bits */ > + ret = cpu_to_le32(*(uint32_t *)tag_mem); > + break; > + case 6: > + /* 256 bytes -> 16 tags -> 64 result bits */ > + return cpu_to_le64(*(uint64_t *)tag_mem); > + default: > + /* cpu configured with invalid gm blocksize. */ > + g_assert_not_reached(); Is it worth having an assert in CPU realize for an invalid blocksize, so that we can catch duff ID register values without having to rely on there being a test run that uses ldgm/stgm ? > } > + shift = extract64(ptr, LOG2_TAG_GRANULE, 4) * 4; > + return ret << shift; > } > > void HELPER(stgm)(CPUARMState *env, uint64_t ptr, uint64_t val) > @@ -462,6 +487,7 @@ void HELPER(stgm)(CPUARMState *env, uint64_t ptr, uint64_t val) > int gm_bs = env_archcpu(env)->gm_blocksize; > int gm_bs_bytes = 4 << gm_bs; > void *tag_mem; > + int shift; > > ptr = QEMU_ALIGN_DOWN(ptr, gm_bs_bytes); > > @@ -480,14 +506,33 @@ void HELPER(stgm)(CPUARMState *env, uint64_t ptr, uint64_t val) > > /* > * The ordering of elements within the word corresponds to > - * a little-endian operation. > + * a little-endian operation. See LDGM for comments on shift. > + * All memory operations are aligned. > */ > + shift = extract64(ptr, LOG2_TAG_GRANULE, 4) * 4; > + val >>= shift; > switch (gm_bs) { > + case 2: > + /* cpu configured with unsupported gm blocksize. */ > + g_assert_not_reached(); > + case 3: > + /* 32 bytes -> 2 tags -> 8 result bits */ > + *(uint8_t *)tag_mem = val; > + break; > + case 4: > + /* 64 bytes -> 4 tags -> 16 result bits */ > + *(uint16_t *)tag_mem = cpu_to_le16(val); > + break; > + case 5: > + /* 128 bytes -> 8 tags -> 32 result bits */ > + *(uint32_t *)tag_mem = cpu_to_le32(val); > + break; > case 6: > - stq_le_p(tag_mem, val); > + /* 256 bytes -> 16 tags -> 64 result bits */ > + *(uint64_t *)tag_mem = cpu_to_le64(val); > break; > default: > - /* cpu configured with unsupported gm blocksize. */ > + /* cpu configured with invalid gm blocksize. */ > g_assert_not_reached(); > } > } > -- > 2.34.1 thanks -- PMM
On 8/10/23 07:23, Peter Maydell wrote: >> + case 4: >> + /* 64 bytes -> 4 tags -> 16 result bits */ >> + ret = cpu_to_le16(*(uint16_t *)tag_mem); > > Does this really make a difference compared to ldw_le_p() ? ldw_le_p uses memcpy, though only mips and sparc hosts do not have unaligned reads, so perhaps it doesn't make much difference. I had originally been thinking about atomicity, but then noticed that the pseudocode uses a loop and so the instruction is therefore non-atomic. > Is it worth having an assert in CPU realize for an invalid > blocksize, so that we can catch duff ID register values > without having to rely on there being a test run that > uses ldgm/stgm ? Yes, that's a good idea. r~
diff --git a/target/arm/tcg/mte_helper.c b/target/arm/tcg/mte_helper.c index 3640c6e57f..6faf4e42d5 100644 --- a/target/arm/tcg/mte_helper.c +++ b/target/arm/tcg/mte_helper.c @@ -428,6 +428,8 @@ uint64_t HELPER(ldgm)(CPUARMState *env, uint64_t ptr) int gm_bs = env_archcpu(env)->gm_blocksize; int gm_bs_bytes = 4 << gm_bs; void *tag_mem; + uint64_t ret; + int shift; ptr = QEMU_ALIGN_DOWN(ptr, gm_bs_bytes); @@ -443,16 +445,39 @@ uint64_t HELPER(ldgm)(CPUARMState *env, uint64_t ptr) /* * The ordering of elements within the word corresponds to - * a little-endian operation. + * a little-endian operation. Computation of shift comes from + * + * index = address<LOG2_TAG_GRANULE+3:LOG2_TAG_GRANULE> + * data<index*4+3:index*4> = tag + * + * Because of the alignment of ptr above, BS=6 has shift=0. + * All memory operations are aligned. */ switch (gm_bs) { - case 6: - /* 256 bytes -> 16 tags -> 64 result bits */ - return ldq_le_p(tag_mem); - default: + case 2: /* cpu configured with unsupported gm blocksize. */ g_assert_not_reached(); + case 3: + /* 32 bytes -> 2 tags -> 8 result bits */ + ret = *(uint8_t *)tag_mem; + break; + case 4: + /* 64 bytes -> 4 tags -> 16 result bits */ + ret = cpu_to_le16(*(uint16_t *)tag_mem); + break; + case 5: + /* 128 bytes -> 8 tags -> 32 result bits */ + ret = cpu_to_le32(*(uint32_t *)tag_mem); + break; + case 6: + /* 256 bytes -> 16 tags -> 64 result bits */ + return cpu_to_le64(*(uint64_t *)tag_mem); + default: + /* cpu configured with invalid gm blocksize. */ + g_assert_not_reached(); } + shift = extract64(ptr, LOG2_TAG_GRANULE, 4) * 4; + return ret << shift; } void HELPER(stgm)(CPUARMState *env, uint64_t ptr, uint64_t val) @@ -462,6 +487,7 @@ void HELPER(stgm)(CPUARMState *env, uint64_t ptr, uint64_t val) int gm_bs = env_archcpu(env)->gm_blocksize; int gm_bs_bytes = 4 << gm_bs; void *tag_mem; + int shift; ptr = QEMU_ALIGN_DOWN(ptr, gm_bs_bytes); @@ -480,14 +506,33 @@ void HELPER(stgm)(CPUARMState *env, uint64_t ptr, uint64_t val) /* * The ordering of elements within the word corresponds to - * a little-endian operation. + * a little-endian operation. See LDGM for comments on shift. + * All memory operations are aligned. */ + shift = extract64(ptr, LOG2_TAG_GRANULE, 4) * 4; + val >>= shift; switch (gm_bs) { + case 2: + /* cpu configured with unsupported gm blocksize. */ + g_assert_not_reached(); + case 3: + /* 32 bytes -> 2 tags -> 8 result bits */ + *(uint8_t *)tag_mem = val; + break; + case 4: + /* 64 bytes -> 4 tags -> 16 result bits */ + *(uint16_t *)tag_mem = cpu_to_le16(val); + break; + case 5: + /* 128 bytes -> 8 tags -> 32 result bits */ + *(uint32_t *)tag_mem = cpu_to_le32(val); + break; case 6: - stq_le_p(tag_mem, val); + /* 256 bytes -> 16 tags -> 64 result bits */ + *(uint64_t *)tag_mem = cpu_to_le64(val); break; default: - /* cpu configured with unsupported gm blocksize. */ + /* cpu configured with invalid gm blocksize. */ g_assert_not_reached(); } }
Support all of the easy GM block sizes. Use direct memory operations, since the pointers are aligned. While BS=2 (16 bytes, 1 tag) is a legal setting, that requires an atomic store of one nibble. This is not difficult, but there is also no point in supporting it until required. Note that cortex-a710 sets GM blocksize to match its cacheline size of 64 bytes. I expect many implementations will also match the cacheline, which makes 16 bytes very unlikely. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/tcg/mte_helper.c | 61 ++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 8 deletions(-)