Switch STE/CD bitfield definitions and accessors to the
'registerfields.h' REG/FIELD API.
FOLLOW-UP: Fix CTXPTR_HI/S2TTB_HI/TTB0_HI/TTB1_HI high bits width
(should be 24 bits, not 16).
Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
include/hw/arm/smmuv3-common.h | 169 +++++++++++++++++++++++----------
1 file changed, 120 insertions(+), 49 deletions(-)
diff --git a/include/hw/arm/smmuv3-common.h b/include/hw/arm/smmuv3-common.h
index 9da817f41a..6b48b5414d 100644
--- a/include/hw/arm/smmuv3-common.h
+++ b/include/hw/arm/smmuv3-common.h
@@ -11,6 +11,8 @@
#ifndef HW_ARM_SMMUV3_COMMON_H
#define HW_ARM_SMMUV3_COMMON_H
+#include "hw/core/registerfields.h"
+
/* Configuration Data */
/* STE Level 1 Descriptor */
@@ -35,63 +37,132 @@ typedef struct CD {
/* STE fields */
-#define STE_VALID(x) extract32((x)->word[0], 0, 1)
+REG32(STE_0, 0)
+ FIELD(STE_0, VALID, 0, 1)
+ FIELD(STE_0, CONFIG, 1, 3)
+ FIELD(STE_0, S1FMT, 4, 2)
+ FIELD(STE_0, CTXPTR_LO, 6, 26)
+REG32(STE_1, 4)
+ FIELD(STE_1, CTXPTR_HI, 0, 16)
+ FIELD(STE_1, S1CDMAX, 27, 5)
+REG32(STE_2, 8)
+ FIELD(STE_2, S1STALLD, 27, 1)
+ FIELD(STE_2, EATS, 28, 2)
+ FIELD(STE_2, STRW, 30, 2)
+REG32(STE_4, 16)
+ FIELD(STE_4, S2VMID, 0, 16)
+REG32(STE_5, 20)
+ FIELD(STE_5, S2T0SZ, 0, 6)
+ FIELD(STE_5, S2SL0, 6, 2)
+ FIELD(STE_5, S2TG, 14, 2)
+ FIELD(STE_5, S2PS, 16, 3)
+ FIELD(STE_5, S2AA64, 19, 1)
+ FIELD(STE_5, S2ENDI, 20, 1)
+ FIELD(STE_5, S2AFFD, 21, 1)
+ FIELD(STE_5, S2HD, 23, 1)
+ FIELD(STE_5, S2HA, 24, 1)
+ FIELD(STE_5, S2S, 25, 1)
+ FIELD(STE_5, S2R, 26, 1)
+REG32(STE_6, 24)
+ FIELD(STE_6, S2TTB_LO, 4, 28)
+REG32(STE_7, 28)
+ FIELD(STE_7, S2TTB_HI, 0, 16)
+
+/* Get STE fields */
+#define STE_VALID(x) FIELD_EX32((x)->word[0], STE_0, VALID)
+#define STE_CONFIG(x) FIELD_EX32((x)->word[0], STE_0, CONFIG)
+#define STE_S1FMT(x) FIELD_EX32((x)->word[0], STE_0, S1FMT)
+#define STE_CTXPTR(x) \
+ (((uint64_t)FIELD_EX32((x)->word[0], STE_0, CTXPTR_LO) << 6) | \
+ ((uint64_t)FIELD_EX32((x)->word[1], STE_1, CTXPTR_HI) << 32))
+#define STE_S1CDMAX(x) FIELD_EX32((x)->word[1], STE_1, S1CDMAX)
+#define STE_S1STALLD(x) FIELD_EX32((x)->word[2], STE_2, S1STALLD)
+#define STE_EATS(x) FIELD_EX32((x)->word[2], STE_2, EATS)
+#define STE_STRW(x) FIELD_EX32((x)->word[2], STE_2, STRW)
+#define STE_S2VMID(x) FIELD_EX32((x)->word[4], STE_4, S2VMID)
+#define STE_S2T0SZ(x) FIELD_EX32((x)->word[5], STE_5, S2T0SZ)
+#define STE_S2SL0(x) FIELD_EX32((x)->word[5], STE_5, S2SL0)
+#define STE_S2TG(x) FIELD_EX32((x)->word[5], STE_5, S2TG)
+#define STE_S2PS(x) FIELD_EX32((x)->word[5], STE_5, S2PS)
+#define STE_S2AA64(x) FIELD_EX32((x)->word[5], STE_5, S2AA64)
+#define STE_S2ENDI(x) FIELD_EX32((x)->word[5], STE_5, S2ENDI)
+#define STE_S2AFFD(x) FIELD_EX32((x)->word[5], STE_5, S2AFFD)
+#define STE_S2HD(x) FIELD_EX32((x)->word[5], STE_5, S2HD)
+#define STE_S2HA(x) FIELD_EX32((x)->word[5], STE_5, S2HA)
+#define STE_S2S(x) FIELD_EX32((x)->word[5], STE_5, S2S)
+#define STE_S2R(x) FIELD_EX32((x)->word[5], STE_5, S2R)
+#define STE_S2TTB(x) \
+ (((uint64_t)FIELD_EX32((x)->word[6], STE_6, S2TTB_LO) << 4) | \
+ ((uint64_t)FIELD_EX32((x)->word[7], STE_7, S2TTB_HI) << 32))
-#define STE_CONFIG(x) extract32((x)->word[0], 1, 3)
#define STE_CFG_S1_ENABLED(config) (config & 0x1)
#define STE_CFG_S2_ENABLED(config) (config & 0x2)
#define STE_CFG_ABORT(config) (!(config & 0x4))
#define STE_CFG_BYPASS(config) (config == 0x4)
-#define STE_S1FMT(x) extract32((x)->word[0], 4 , 2)
-#define STE_S1CDMAX(x) extract32((x)->word[1], 27, 5)
-#define STE_S1STALLD(x) extract32((x)->word[2], 27, 1)
-#define STE_EATS(x) extract32((x)->word[2], 28, 2)
-#define STE_STRW(x) extract32((x)->word[2], 30, 2)
-#define STE_S2VMID(x) extract32((x)->word[4], 0 , 16)
-#define STE_S2T0SZ(x) extract32((x)->word[5], 0 , 6)
-#define STE_S2SL0(x) extract32((x)->word[5], 6 , 2)
-#define STE_S2TG(x) extract32((x)->word[5], 14, 2)
-#define STE_S2PS(x) extract32((x)->word[5], 16, 3)
-#define STE_S2AA64(x) extract32((x)->word[5], 19, 1)
-#define STE_S2ENDI(x) extract32((x)->word[5], 20, 1)
-#define STE_S2AFFD(x) extract32((x)->word[5], 21, 1)
-#define STE_S2HD(x) extract32((x)->word[5], 23, 1)
-#define STE_S2HA(x) extract32((x)->word[5], 24, 1)
-#define STE_S2S(x) extract32((x)->word[5], 25, 1)
-#define STE_S2R(x) extract32((x)->word[5], 26, 1)
-
-#define STE_CTXPTR(x) \
- ((extract64((x)->word[1], 0, 16) << 32) | \
- ((x)->word[0] & 0xffffffc0))
-
-#define STE_S2TTB(x) \
- ((extract64((x)->word[7], 0, 16) << 32) | \
- ((x)->word[6] & 0xfffffff0))
-
/* CD fields */
-#define CD_VALID(x) extract32((x)->word[0], 31, 1)
-#define CD_ASID(x) extract32((x)->word[1], 16, 16)
-#define CD_TTB(x, sel) \
- ((extract64((x)->word[(sel) * 2 + 3], 0, 19) << 32) | \
- ((x)->word[(sel) * 2 + 2] & ~0xfULL))
-
-#define CD_HAD(x, sel) extract32((x)->word[(sel) * 2 + 2], 1, 1)
-
-#define CD_TSZ(x, sel) extract32((x)->word[0], (16 * (sel)) + 0, 6)
-#define CD_TG(x, sel) extract32((x)->word[0], (16 * (sel)) + 6, 2)
-#define CD_EPD(x, sel) extract32((x)->word[0], (16 * (sel)) + 14, 1)
-#define CD_ENDI(x) extract32((x)->word[0], 15, 1)
-#define CD_IPS(x) extract32((x)->word[1], 0 , 3)
-#define CD_AFFD(x) extract32((x)->word[1], 3 , 1)
-#define CD_TBI(x) extract32((x)->word[1], 6 , 2)
-#define CD_HD(x) extract32((x)->word[1], 10 , 1)
-#define CD_HA(x) extract32((x)->word[1], 11 , 1)
-#define CD_S(x) extract32((x)->word[1], 12, 1)
-#define CD_R(x) extract32((x)->word[1], 13, 1)
-#define CD_A(x) extract32((x)->word[1], 14, 1)
-#define CD_AARCH64(x) extract32((x)->word[1], 9 , 1)
+REG32(CD_0, 0)
+ FIELD(CD_0, TSZ0, 0, 6)
+ FIELD(CD_0, TG0, 6, 2)
+ FIELD(CD_0, EPD0, 14, 1)
+ FIELD(CD_0, ENDI, 15, 1)
+ FIELD(CD_0, TSZ1, 16, 6)
+ FIELD(CD_0, TG1, 22, 2)
+ FIELD(CD_0, EPD1, 30, 1)
+ FIELD(CD_0, VALID, 31, 1)
+REG32(CD_1, 4)
+ FIELD(CD_1, IPS, 0, 3)
+ FIELD(CD_1, AFFD, 3, 1)
+ FIELD(CD_1, TBI, 6, 2)
+ FIELD(CD_1, AARCH64, 9, 1)
+ FIELD(CD_1, HD, 10, 1)
+ FIELD(CD_1, HA, 11, 1)
+ FIELD(CD_1, S, 12, 1)
+ FIELD(CD_1, R, 13, 1)
+ FIELD(CD_1, A, 14, 1)
+ FIELD(CD_1, ASID, 16, 16)
+REG32(CD_2, 8)
+ FIELD(CD_2, HAD0, 1, 1)
+ FIELD(CD_2, TTB0_LO, 4, 28)
+REG32(CD_3, 12)
+ FIELD(CD_3, TTB0_HI, 0, 19)
+REG32(CD_4, 16)
+ FIELD(CD_4, HAD1, 1, 1)
+ FIELD(CD_4, TTB1_LO, 4, 28)
+REG32(CD_5, 20)
+ FIELD(CD_5, TTB1_HI, 0, 19)
+
+/* Get CD fields */
+#define CD_TSZ(x, sel) ((sel) ? \
+ FIELD_EX32((x)->word[0], CD_0, TSZ1) : \
+ FIELD_EX32((x)->word[0], CD_0, TSZ0))
+#define CD_TG(x, sel) ((sel) ? \
+ FIELD_EX32((x)->word[0], CD_0, TG1) : \
+ FIELD_EX32((x)->word[0], CD_0, TG0))
+#define CD_EPD(x, sel) ((sel) ? \
+ FIELD_EX32((x)->word[0], CD_0, EPD1) : \
+ FIELD_EX32((x)->word[0], CD_0, EPD0))
+#define CD_ENDI(x) FIELD_EX32((x)->word[0], CD_0, ENDI)
+#define CD_VALID(x) FIELD_EX32((x)->word[0], CD_0, VALID)
+#define CD_IPS(x) FIELD_EX32((x)->word[1], CD_1, IPS)
+#define CD_AFFD(x) FIELD_EX32((x)->word[1], CD_1, AFFD)
+#define CD_TBI(x) FIELD_EX32((x)->word[1], CD_1, TBI)
+#define CD_AARCH64(x) FIELD_EX32((x)->word[1], CD_1, AARCH64)
+#define CD_HD(x) FIELD_EX32((x)->word[1], CD_1, HD)
+#define CD_HA(x) FIELD_EX32((x)->word[1], CD_1, HA)
+#define CD_S(x) FIELD_EX32((x)->word[1], CD_1, S)
+#define CD_R(x) FIELD_EX32((x)->word[1], CD_1, R)
+#define CD_A(x) FIELD_EX32((x)->word[1], CD_1, A)
+#define CD_ASID(x) FIELD_EX32((x)->word[1], CD_1, ASID)
+#define CD_HAD(x, sel) ((sel) ? \
+ FIELD_EX32((x)->word[4], CD_4, HAD1) : \
+ FIELD_EX32((x)->word[2], CD_2, HAD0))
+#define CD_TTB(x, sel) \
+ ((sel) ? (((uint64_t)FIELD_EX32((x)->word[5], CD_5, TTB1_HI) << 32) | \
+ ((uint64_t)FIELD_EX32((x)->word[4], CD_4, TTB1_LO) << 4)) : \
+ (((uint64_t)FIELD_EX32((x)->word[3], CD_3, TTB0_HI) << 32) | \
+ ((uint64_t)FIELD_EX32((x)->word[2], CD_2, TTB0_LO) << 4)))
/* MMIO Registers */
--
2.34.1On 19/1/26 17:11, Tao Tang wrote: > Switch STE/CD bitfield definitions and accessors to the > 'registerfields.h' REG/FIELD API. > > FOLLOW-UP: Fix CTXPTR_HI/S2TTB_HI/TTB0_HI/TTB1_HI high bits width > (should be 24 bits, not 16). Right, but ... > Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn> > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > Reviewed-by: Eric Auger <eric.auger@redhat.com> > --- > include/hw/arm/smmuv3-common.h | 169 +++++++++++++++++++++++---------- > 1 file changed, 120 insertions(+), 49 deletions(-) > -#define STE_VALID(x) extract32((x)->word[0], 0, 1) > +REG32(STE_0, 0) > + FIELD(STE_0, VALID, 0, 1) > + FIELD(STE_0, CONFIG, 1, 3) > + FIELD(STE_0, S1FMT, 4, 2) > + FIELD(STE_0, CTXPTR_LO, 6, 26) > +REG32(STE_1, 4) > + FIELD(STE_1, CTXPTR_HI, 0, 16) ... not followed up?
Hi Philippe, On 2026/1/20 00:38, Philippe Mathieu-Daudé wrote: > On 19/1/26 17:11, Tao Tang wrote: >> Switch STE/CD bitfield definitions and accessors to the >> 'registerfields.h' REG/FIELD API. >> >> FOLLOW-UP: Fix CTXPTR_HI/S2TTB_HI/TTB0_HI/TTB1_HI high bits width >> (should be 24 bits, not 16). > > Right, but ... > >> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn> >> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> Reviewed-by: Eric Auger <eric.auger@redhat.com> >> --- >> include/hw/arm/smmuv3-common.h | 169 +++++++++++++++++++++++---------- >> 1 file changed, 120 insertions(+), 49 deletions(-) > > >> -#define STE_VALID(x) extract32((x)->word[0], 0, 1) >> +REG32(STE_0, 0) >> + FIELD(STE_0, VALID, 0, 1) >> + FIELD(STE_0, CONFIG, 1, 3) >> + FIELD(STE_0, S1FMT, 4, 2) >> + FIELD(STE_0, CTXPTR_LO, 6, 26) >> +REG32(STE_1, 4) >> + FIELD(STE_1, CTXPTR_HI, 0, 16) > > ... not followed up? Sorry, there's a typo here. What I was thinking at the time was that I would submit a separate patch follow this series. Do you need me to resend this series? Best regards, Tao
On 19/1/26 17:51, Tao Tang wrote:
> Hi Philippe,
>
> On 2026/1/20 00:38, Philippe Mathieu-Daudé wrote:
>> On 19/1/26 17:11, Tao Tang wrote:
>>> Switch STE/CD bitfield definitions and accessors to the
>>> 'registerfields.h' REG/FIELD API.
>>>
>>> FOLLOW-UP: Fix CTXPTR_HI/S2TTB_HI/TTB0_HI/TTB1_HI high bits width
>>> (should be 24 bits, not 16).
>>
>> Right, but ...
>>
>>> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>>> ---
>>> include/hw/arm/smmuv3-common.h | 169 +++++++++++++++++++++++----------
>>> 1 file changed, 120 insertions(+), 49 deletions(-)
>>
>>
>>> -#define STE_VALID(x) extract32((x)->word[0], 0, 1)
>>> +REG32(STE_0, 0)
>>> + FIELD(STE_0, VALID, 0, 1)
>>> + FIELD(STE_0, CONFIG, 1, 3)
>>> + FIELD(STE_0, S1FMT, 4, 2)
>>> + FIELD(STE_0, CTXPTR_LO, 6, 26)
>>> +REG32(STE_1, 4)
>>> + FIELD(STE_1, CTXPTR_HI, 0, 16)
>>
>> ... not followed up?
>
>
> Sorry, there's a typo here. What I was thinking at the time was that I
> would submit a separate patch follow this series.
I squashed:
-- >8 --
diff --git a/include/hw/arm/smmuv3-common.h b/include/hw/arm/smmuv3-common.h
index 6b48b5414dd..db30331441a 100644
--- a/include/hw/arm/smmuv3-common.h
+++ b/include/hw/arm/smmuv3-common.h
@@ -43,7 +43,7 @@ REG32(STE_0, 0)
FIELD(STE_0, S1FMT, 4, 2)
FIELD(STE_0, CTXPTR_LO, 6, 26)
REG32(STE_1, 4)
- FIELD(STE_1, CTXPTR_HI, 0, 16)
+ FIELD(STE_1, CTXPTR_HI, 0, 24)
FIELD(STE_1, S1CDMAX, 27, 5)
REG32(STE_2, 8)
FIELD(STE_2, S1STALLD, 27, 1)
(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? y
@@ -66,7 +66,7 @@ REG32(STE_5, 20)
REG32(STE_6, 24)
FIELD(STE_6, S2TTB_LO, 4, 28)
REG32(STE_7, 28)
- FIELD(STE_7, S2TTB_HI, 0, 16)
+ FIELD(STE_7, S2TTB_HI, 0, 24)
---
Is that OK with you? Do TTB0_HI/TTB1_HI need update too?
>
> Do you need me to resend this series?
Hopefully no :)
Regards,
Phil.
Hi Philippe, On 2026/1/20 01:22, Philippe Mathieu-Daudé wrote: > On 19/1/26 17:51, Tao Tang wrote: >> Hi Philippe, >> >> On 2026/1/20 00:38, Philippe Mathieu-Daudé wrote: >>> On 19/1/26 17:11, Tao Tang wrote: >>>> Switch STE/CD bitfield definitions and accessors to the >>>> 'registerfields.h' REG/FIELD API. >>>> >>>> FOLLOW-UP: Fix CTXPTR_HI/S2TTB_HI/TTB0_HI/TTB1_HI high bits width >>>> (should be 24 bits, not 16). >>> >>> Right, but ... >>> >>>> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn> >>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>>> Reviewed-by: Eric Auger <eric.auger@redhat.com> >>>> --- >>>> include/hw/arm/smmuv3-common.h | 169 >>>> +++++++++++++++++++++++---------- >>>> 1 file changed, 120 insertions(+), 49 deletions(-) >>> >>> >>>> -#define STE_VALID(x) extract32((x)->word[0], 0, 1) >>>> +REG32(STE_0, 0) >>>> + FIELD(STE_0, VALID, 0, 1) >>>> + FIELD(STE_0, CONFIG, 1, 3) >>>> + FIELD(STE_0, S1FMT, 4, 2) >>>> + FIELD(STE_0, CTXPTR_LO, 6, 26) >>>> +REG32(STE_1, 4) >>>> + FIELD(STE_1, CTXPTR_HI, 0, 16) >>> >>> ... not followed up? >> >> >> Sorry, there's a typo here. What I was thinking at the time was that >> I would submit a separate patch follow this series. > > I squashed: > > -- >8 -- > diff --git a/include/hw/arm/smmuv3-common.h > b/include/hw/arm/smmuv3-common.h > index 6b48b5414dd..db30331441a 100644 > --- a/include/hw/arm/smmuv3-common.h > +++ b/include/hw/arm/smmuv3-common.h > @@ -43,7 +43,7 @@ REG32(STE_0, 0) > FIELD(STE_0, S1FMT, 4, 2) > FIELD(STE_0, CTXPTR_LO, 6, 26) > REG32(STE_1, 4) > - FIELD(STE_1, CTXPTR_HI, 0, 16) > + FIELD(STE_1, CTXPTR_HI, 0, 24) > FIELD(STE_1, S1CDMAX, 27, 5) > REG32(STE_2, 8) > FIELD(STE_2, S1STALLD, 27, 1) > (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? y > @@ -66,7 +66,7 @@ REG32(STE_5, 20) > REG32(STE_6, 24) > FIELD(STE_6, S2TTB_LO, 4, 28) > REG32(STE_7, 28) > - FIELD(STE_7, S2TTB_HI, 0, 16) > + FIELD(STE_7, S2TTB_HI, 0, 24) > > --- > > Is that OK with you? Do TTB0_HI/TTB1_HI need update too? Yes we should update TTB0_HI/TTB1_HI too. A total of four modifications are needed here: CTXPTR_HI S2TTB_HI TTB0_HI TTB1_HI Thanks, Tao
Hi Tao,
On 20/1/26 02:02, Tao Tang wrote:
> On 2026/1/20 01:22, Philippe Mathieu-Daudé wrote:
>> On 19/1/26 17:51, Tao Tang wrote:
>>> Hi Philippe,
>>>
>>> On 2026/1/20 00:38, Philippe Mathieu-Daudé wrote:
>>>> On 19/1/26 17:11, Tao Tang wrote:
>>>>> Switch STE/CD bitfield definitions and accessors to the
>>>>> 'registerfields.h' REG/FIELD API.
>>>>>
>>>>> FOLLOW-UP: Fix CTXPTR_HI/S2TTB_HI/TTB0_HI/TTB1_HI high bits width
>>>>> (should be 24 bits, not 16).
>>>>
>>>> Right, but ...
>>>>
>>>>> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
>>>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>>>>> ---
>>>>> include/hw/arm/smmuv3-common.h | 169 ++++++++++++++++++++++
>>>>> +----------
>>>>> 1 file changed, 120 insertions(+), 49 deletions(-)
>>>>
>>>>
>>>>> -#define STE_VALID(x) extract32((x)->word[0], 0, 1)
>>>>> +REG32(STE_0, 0)
>>>>> + FIELD(STE_0, VALID, 0, 1)
>>>>> + FIELD(STE_0, CONFIG, 1, 3)
>>>>> + FIELD(STE_0, S1FMT, 4, 2)
>>>>> + FIELD(STE_0, CTXPTR_LO, 6, 26)
>>>>> +REG32(STE_1, 4)
>>>>> + FIELD(STE_1, CTXPTR_HI, 0, 16)
>>>>
>>>> ... not followed up?
>>>
>>>
>>> Sorry, there's a typo here. What I was thinking at the time was that
>>> I would submit a separate patch follow this series.
>>
>> I squashed:
>>
>> -- >8 --
>> diff --git a/include/hw/arm/smmuv3-common.h b/include/hw/arm/smmuv3-
>> common.h
>> index 6b48b5414dd..db30331441a 100644
>> --- a/include/hw/arm/smmuv3-common.h
>> +++ b/include/hw/arm/smmuv3-common.h
>> @@ -43,7 +43,7 @@ REG32(STE_0, 0)
>> FIELD(STE_0, S1FMT, 4, 2)
>> FIELD(STE_0, CTXPTR_LO, 6, 26)
>> REG32(STE_1, 4)
>> - FIELD(STE_1, CTXPTR_HI, 0, 16)
>> + FIELD(STE_1, CTXPTR_HI, 0, 24)
>> FIELD(STE_1, S1CDMAX, 27, 5)
>> REG32(STE_2, 8)
>> FIELD(STE_2, S1STALLD, 27, 1)
>> (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? y
>> @@ -66,7 +66,7 @@ REG32(STE_5, 20)
>> REG32(STE_6, 24)
>> FIELD(STE_6, S2TTB_LO, 4, 28)
>> REG32(STE_7, 28)
>> - FIELD(STE_7, S2TTB_HI, 0, 16)
>> + FIELD(STE_7, S2TTB_HI, 0, 24)
>>
>> ---
>>
>> Is that OK with you? Do TTB0_HI/TTB1_HI need update too?
>
> Yes we should update TTB0_HI/TTB1_HI too. A total of four modifications
> are needed here:
>
> CTXPTR_HI
>
> S2TTB_HI
>
> TTB0_HI
>
> TTB1_HI
I am just trying to help you getting your series merged. I know this is
v9 and you might be frustrated, but simply providing the correct 4
values would have helped both of us and save further exchanges.
In v8 Eric said to change 16 -> 24 for 4 fields and you confirmed:
https://lore.kernel.org/qemu-devel/2255d30c-cc58-4a45-974c-f7fd89694c26@phytium.com.cn/
Looking at ARM IHI 0070G.b this is only valid for CTXPTR_HI, for the
rest only 20 bits remains, 4 are RES0 (SMMUv3.1).
It would have saved me some hours checking the spec if you had
provided the correct values beforehand. I'm going to squash the
following instead:
-- >8 --
diff --git a/include/hw/arm/smmuv3-common.h b/include/hw/arm/smmuv3-common.h
index 6b48b5414dd..f0e1dd85715 100644
--- a/include/hw/arm/smmuv3-common.h
+++ b/include/hw/arm/smmuv3-common.h
@@ -45,3 +45,3 @@ REG32(STE_0, 0)
REG32(STE_1, 4)
- FIELD(STE_1, CTXPTR_HI, 0, 16)
+ FIELD(STE_1, CTXPTR_HI, 0, 24)
FIELD(STE_1, S1CDMAX, 27, 5)
@@ -68,3 +68,3 @@ REG32(STE_6, 24)
REG32(STE_7, 28)
- FIELD(STE_7, S2TTB_HI, 0, 16)
+ FIELD(STE_7, S2TTB_HI, 0, 20)
@@ -128,3 +128,3 @@ REG32(CD_2, 8)
REG32(CD_3, 12)
- FIELD(CD_3, TTB0_HI, 0, 19)
+ FIELD(CD_3, TTB0_HI, 0, 20)
REG32(CD_4, 16)
@@ -133,3 +133,3 @@ REG32(CD_4, 16)
REG32(CD_5, 20)
- FIELD(CD_5, TTB1_HI, 0, 19)
+ FIELD(CD_5, TTB1_HI, 0, 20)
---
If you disagree, please post a patch on top to correct.
Regards,
Phil.
Hi Philippe, On 2026/1/21 01:57, Philippe Mathieu-Daudé wrote: > Hi Tao, > > On 20/1/26 02:02, Tao Tang wrote: >> On 2026/1/20 01:22, Philippe Mathieu-Daudé wrote: >>> On 19/1/26 17:51, Tao Tang wrote: >>>> Hi Philippe, >>>> >>>> On 2026/1/20 00:38, Philippe Mathieu-Daudé wrote: >>>>> On 19/1/26 17:11, Tao Tang wrote: >>>>>> Switch STE/CD bitfield definitions and accessors to the >>>>>> 'registerfields.h' REG/FIELD API. >>>>>> >>>>>> FOLLOW-UP: Fix CTXPTR_HI/S2TTB_HI/TTB0_HI/TTB1_HI high bits width >>>>>> (should be 24 bits, not 16). >>>>> >>>>> Right, but ... >>>>> >>>>>> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn> >>>>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>>>>> Reviewed-by: Eric Auger <eric.auger@redhat.com> >>>>>> --- >>>>>> include/hw/arm/smmuv3-common.h | 169 ++++++++++++++++++++++ >>>>>> +---------- >>>>>> 1 file changed, 120 insertions(+), 49 deletions(-) >>>>> >>>>> >>>>>> -#define STE_VALID(x) extract32((x)->word[0], 0, 1) >>>>>> +REG32(STE_0, 0) >>>>>> + FIELD(STE_0, VALID, 0, 1) >>>>>> + FIELD(STE_0, CONFIG, 1, 3) >>>>>> + FIELD(STE_0, S1FMT, 4, 2) >>>>>> + FIELD(STE_0, CTXPTR_LO, 6, 26) >>>>>> +REG32(STE_1, 4) >>>>>> + FIELD(STE_1, CTXPTR_HI, 0, 16) >>>>> >>>>> ... not followed up? >>>> >>>> >>>> Sorry, there's a typo here. What I was thinking at the time was >>>> that I would submit a separate patch follow this series. >>> >>> I squashed: >>> >>> -- >8 -- >>> diff --git a/include/hw/arm/smmuv3-common.h b/include/hw/arm/smmuv3- >>> common.h >>> index 6b48b5414dd..db30331441a 100644 >>> --- a/include/hw/arm/smmuv3-common.h >>> +++ b/include/hw/arm/smmuv3-common.h >>> @@ -43,7 +43,7 @@ REG32(STE_0, 0) >>> FIELD(STE_0, S1FMT, 4, 2) >>> FIELD(STE_0, CTXPTR_LO, 6, 26) >>> REG32(STE_1, 4) >>> - FIELD(STE_1, CTXPTR_HI, 0, 16) >>> + FIELD(STE_1, CTXPTR_HI, 0, 24) >>> FIELD(STE_1, S1CDMAX, 27, 5) >>> REG32(STE_2, 8) >>> FIELD(STE_2, S1STALLD, 27, 1) >>> (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? y >>> @@ -66,7 +66,7 @@ REG32(STE_5, 20) >>> REG32(STE_6, 24) >>> FIELD(STE_6, S2TTB_LO, 4, 28) >>> REG32(STE_7, 28) >>> - FIELD(STE_7, S2TTB_HI, 0, 16) >>> + FIELD(STE_7, S2TTB_HI, 0, 24) >>> >>> --- >>> >>> Is that OK with you? Do TTB0_HI/TTB1_HI need update too? >> >> Yes we should update TTB0_HI/TTB1_HI too. A total of four >> modifications are needed here: >> >> CTXPTR_HI >> >> S2TTB_HI >> >> TTB0_HI >> >> TTB1_HI > > I am just trying to help you getting your series merged. I know this is > v9 and you might be frustrated, but simply providing the correct 4 > values would have helped both of us and save further exchanges. > > In v8 Eric said to change 16 -> 24 for 4 fields and you confirmed: > https://lore.kernel.org/qemu-devel/2255d30c-cc58-4a45-974c-f7fd89694c26@phytium.com.cn/ > > > Looking at ARM IHI 0070G.b this is only valid for CTXPTR_HI, for the > rest only 20 bits remains, 4 are RES0 (SMMUv3.1). > > It would have saved me some hours checking the spec if you had > provided the correct values beforehand. I'm going to squash the > following instead: > > -- >8 -- > diff --git a/include/hw/arm/smmuv3-common.h > b/include/hw/arm/smmuv3-common.h > index 6b48b5414dd..f0e1dd85715 100644 > --- a/include/hw/arm/smmuv3-common.h > +++ b/include/hw/arm/smmuv3-common.h > @@ -45,3 +45,3 @@ REG32(STE_0, 0) > REG32(STE_1, 4) > - FIELD(STE_1, CTXPTR_HI, 0, 16) > + FIELD(STE_1, CTXPTR_HI, 0, 24) > FIELD(STE_1, S1CDMAX, 27, 5) > @@ -68,3 +68,3 @@ REG32(STE_6, 24) > REG32(STE_7, 28) > - FIELD(STE_7, S2TTB_HI, 0, 16) > + FIELD(STE_7, S2TTB_HI, 0, 20) > > @@ -128,3 +128,3 @@ REG32(CD_2, 8) > REG32(CD_3, 12) > - FIELD(CD_3, TTB0_HI, 0, 19) > + FIELD(CD_3, TTB0_HI, 0, 20) > REG32(CD_4, 16) > @@ -133,3 +133,3 @@ REG32(CD_4, 16) > REG32(CD_5, 20) > - FIELD(CD_5, TTB1_HI, 0, 19) > + FIELD(CD_5, TTB1_HI, 0, 20) > > --- > > If you disagree, please post a patch on top to correct. > > Regards, > > Phil. Sorry I wasn’t clear earlier and didn’t provide the exact values and reasoning — that wasted your time. Regarding the spec versions, F.b and earlier used narrower ranges (e.g. CD.TTB0 [115:68]), while from G.a onward these base-pointer fields are shown with the widened ranges (e.g. CD.TTB0 [119:68]). For clarity, the main layout changes I was referring to from F.b to G.a are: STE.S1ContextPtr [51:6] -> [55:6] STE.S2TTB [243:196] -> [247:196] CD.TTB0 [115:68] -> [119:68] CD.TTB1 [179:132] -> [183:132] (Where my earlier [115:68] example was specifically for TTB0.) - G.a: https://developer.arm.com/documentation/ihi0070/ga/?lang=en - F.b: https://developer.arm.com/documentation/ihi0070/fb/?lang=en I initially looked only at the layout/bit ranges (5.4 CD) and missed that the top 4 bits are only meaningful when AA64 selects VMSAv9-128; otherwise they are RES0. With that in mind, I agree with your updated widths and I’m OK with you squashing the changes as proposed. Best regards, Tao
© 2016 - 2026 Red Hat, Inc.