get_phys_addr_disabled() is called from only one function,
get_phys_addr_nogpc(), and from two locations within it.

The first call to get_phys_addr_disabled() occurs when mmu_idx is one of
the following: ARMMMUIdx_Phys_S, ARMMMUIdx_Phys_NS, ARMMMUIdx_Phys_Root,
or ARMMMUIdx_Phys_Realm. So, handling ARMMMUIdx_Stage2 or
ARMMMUIdx_Stage2_S is not required in get_phys_addr_disabled() for this
case.

The second call to get_phys_addr_disabled(), with mmu_idx ==
ARMMMUIdx_Stage2 (or ARMMMUIdx_Stage2_S), only would occur if
regime_translation_disabled() returns
true for these mmu indexes. However, mmu_idx == ARMMMUIdx_Stage2 (or
ARMMMUIdx_Stage2_S) can only occur if get_phys_addr_twostage() was
called, since it's the only place where ptw->in_mmu_idx is set to
ARMMMUIdx_Stage2 (or ARMMMUIdx_Stage2_S) and that only happens if
regime_translation_disabled() returns false.

Therefore, at this second call site, get_phys_addr_disabled() is
never invoked with mmu_idx == ARMMMUIdx_Stage2 or ARMMMUIdx_Stage2_S.

Hence, since get_phys_addr_disabled() can never be called with
mmu_idx == ARMMMUIdx_Stage2 or ARMMMUIdx_Stage2_S, removed these two
cases from this function.

CI: https://gitlab.com/gusbromero/qemu/-/pipelines/2335353022

The failure in the "migration-compat-aarch64" test is also observed on
"master", so the chance it's caused by this cleanup is very low. Also,
it's a test marked as "allowed to fail".

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
 target/arm/ptw.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 8b8dc09e72..b8a3150f14 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -3449,8 +3449,6 @@ static bool get_phys_addr_disabled(CPUARMState *env,
     int r_el;
 
     switch (mmu_idx) {
-    case ARMMMUIdx_Stage2:
-    case ARMMMUIdx_Stage2_S:
     case ARMMMUIdx_Phys_S:
     case ARMMMUIdx_Phys_NS:
     case ARMMMUIdx_Phys_Root:
-- 
2.34.1

On Thu, 19 Feb 2026 at 14:02, Gustavo Romero <gustavo.romero@linaro.org> wrote:
>
> get_phys_addr_disabled() is called from only one function,
> get_phys_addr_nogpc(), and from two locations within it.
>
> The first call to get_phys_addr_disabled() occurs when mmu_idx is one of
> the following: ARMMMUIdx_Phys_S, ARMMMUIdx_Phys_NS, ARMMMUIdx_Phys_Root,
> or ARMMMUIdx_Phys_Realm. So, handling ARMMMUIdx_Stage2 or
> ARMMMUIdx_Stage2_S is not required in get_phys_addr_disabled() for this
> case.
>
> The second call to get_phys_addr_disabled(), with mmu_idx ==
> ARMMMUIdx_Stage2 (or ARMMMUIdx_Stage2_S), only would occur if
> regime_translation_disabled() returns
> true for these mmu indexes. However, mmu_idx == ARMMMUIdx_Stage2 (or
> ARMMMUIdx_Stage2_S) can only occur if get_phys_addr_twostage() was
> called, since it's the only place where ptw->in_mmu_idx is set to
> ARMMMUIdx_Stage2 (or ARMMMUIdx_Stage2_S) and that only happens if
> regime_translation_disabled() returns false.

This is true, but only because we never call get_phys_addr_for_at()
with an mmu_idx argument of Stage2, right?

-- PMM

Hi Peter!

On 2/19/26 11:13, Peter Maydell wrote:
> On Thu, 19 Feb 2026 at 14:02, Gustavo Romero <gustavo.romero@linaro.org> wrote:
>>
>> get_phys_addr_disabled() is called from only one function,
>> get_phys_addr_nogpc(), and from two locations within it.
>>
>> The first call to get_phys_addr_disabled() occurs when mmu_idx is one of
>> the following: ARMMMUIdx_Phys_S, ARMMMUIdx_Phys_NS, ARMMMUIdx_Phys_Root,
>> or ARMMMUIdx_Phys_Realm. So, handling ARMMMUIdx_Stage2 or
>> ARMMMUIdx_Stage2_S is not required in get_phys_addr_disabled() for this
>> case.
>>
>> The second call to get_phys_addr_disabled(), with mmu_idx ==
>> ARMMMUIdx_Stage2 (or ARMMMUIdx_Stage2_S), only would occur if
>> regime_translation_disabled() returns
>> true for these mmu indexes. However, mmu_idx == ARMMMUIdx_Stage2 (or
>> ARMMMUIdx_Stage2_S) can only occur if get_phys_addr_twostage() was
>> called, since it's the only place where ptw->in_mmu_idx is set to
>> ARMMMUIdx_Stage2 (or ARMMMUIdx_Stage2_S) and that only happens if
>> regime_translation_disabled() returns false.
> 
> This is true, but only because we never call get_phys_addr_for_at()
> with an mmu_idx argument of Stage2, right?

Correct, and that's a case that will never exist too afaics.


Cheers,
Gustavo

On Thu, 19 Feb 2026 at 15:16, Gustavo Romero <gustavo.romero@linaro.org> wrote:
>
> Hi Peter!
>
> On 2/19/26 11:13, Peter Maydell wrote:
> > On Thu, 19 Feb 2026 at 14:02, Gustavo Romero <gustavo.romero@linaro.org> wrote:
> >>
> >> get_phys_addr_disabled() is called from only one function,
> >> get_phys_addr_nogpc(), and from two locations within it.
> >>
> >> The first call to get_phys_addr_disabled() occurs when mmu_idx is one of
> >> the following: ARMMMUIdx_Phys_S, ARMMMUIdx_Phys_NS, ARMMMUIdx_Phys_Root,
> >> or ARMMMUIdx_Phys_Realm. So, handling ARMMMUIdx_Stage2 or
> >> ARMMMUIdx_Stage2_S is not required in get_phys_addr_disabled() for this
> >> case.
> >>
> >> The second call to get_phys_addr_disabled(), with mmu_idx ==
> >> ARMMMUIdx_Stage2 (or ARMMMUIdx_Stage2_S), only would occur if
> >> regime_translation_disabled() returns
> >> true for these mmu indexes. However, mmu_idx == ARMMMUIdx_Stage2 (or
> >> ARMMMUIdx_Stage2_S) can only occur if get_phys_addr_twostage() was
> >> called, since it's the only place where ptw->in_mmu_idx is set to
> >> ARMMMUIdx_Stage2 (or ARMMMUIdx_Stage2_S) and that only happens if
> >> regime_translation_disabled() returns false.
> >
> > This is true, but only because we never call get_phys_addr_for_at()
> > with an mmu_idx argument of Stage2, right?
>
> Correct, and that's a case that will never exist too afaics.

Well, it doesn't exist currently (because the architecture doesn't
define AT operations that do stage-2-only lookups). But two extra case
labels in one function seems like a fairly small price for "the
get_phys_addr_for_at() function will work correctly if you ask it to
do a stage-2 translation", if we ever need it in future (given that
it's a function ptw.c exposes to the rest of the code).

Also, removing these lines means that if we ever do get here then
we'll drop into the definitely-wrong stage1 handling default case.

-- PMM

Hi Peter,

On 2/19/26 13:13, Peter Maydell wrote:
> On Thu, 19 Feb 2026 at 15:16, Gustavo Romero <gustavo.romero@linaro.org> wrote:
>>
>> Hi Peter!
>>
>> On 2/19/26 11:13, Peter Maydell wrote:
>>> On Thu, 19 Feb 2026 at 14:02, Gustavo Romero <gustavo.romero@linaro.org> wrote:
>>>>
>>>> get_phys_addr_disabled() is called from only one function,
>>>> get_phys_addr_nogpc(), and from two locations within it.
>>>>
>>>> The first call to get_phys_addr_disabled() occurs when mmu_idx is one of
>>>> the following: ARMMMUIdx_Phys_S, ARMMMUIdx_Phys_NS, ARMMMUIdx_Phys_Root,
>>>> or ARMMMUIdx_Phys_Realm. So, handling ARMMMUIdx_Stage2 or
>>>> ARMMMUIdx_Stage2_S is not required in get_phys_addr_disabled() for this
>>>> case.
>>>>
>>>> The second call to get_phys_addr_disabled(), with mmu_idx ==
>>>> ARMMMUIdx_Stage2 (or ARMMMUIdx_Stage2_S), only would occur if
>>>> regime_translation_disabled() returns
>>>> true for these mmu indexes. However, mmu_idx == ARMMMUIdx_Stage2 (or
>>>> ARMMMUIdx_Stage2_S) can only occur if get_phys_addr_twostage() was
>>>> called, since it's the only place where ptw->in_mmu_idx is set to
>>>> ARMMMUIdx_Stage2 (or ARMMMUIdx_Stage2_S) and that only happens if
>>>> regime_translation_disabled() returns false.
>>>
>>> This is true, but only because we never call get_phys_addr_for_at()
>>> with an mmu_idx argument of Stage2, right?
>>
>> Correct, and that's a case that will never exist too afaics.
> 
> Well, it doesn't exist currently (because the architecture doesn't
> define AT operations that do stage-2-only lookups). But two extra case
> labels in one function seems like a fairly small price for "the
> get_phys_addr_for_at() function will work correctly if you ask it to
> do a stage-2 translation", if we ever need it in future (given that
> it's a function ptw.c exposes to the rest of the code).

It doesn't exist currently and in my view it will probably never exist
because Stage2 is not an independent translation regime. It only exists
as an "extension" of a Stage1 translation. Also, Stage2 never translates
VA->PA, but IPA->PA, and IPA is not a valid architectural address.

These two extra cases, being unused, to me, complicates a further bit
the reading of ptw.c. Moreover, the semantics looks misleading, being it
from an AT command or from a real translation, because to fall into
that case regime_translation_disabled() must return true for Stage2,
i.e. Stage2 is disabled -- (hcr_el2 & (HCR_DC | HCR_VM)) == 0, but then
we case for Stage2 anyways in get_phys_addr_disabled().


> Also, removing these lines means that if we ever do get here then
> we'll drop into the definitely-wrong stage1 handling default case.

Getting here with mmu_idx = ARMMMUIdx_Stage2{,S} looks, first of
all, semantically wrong to me, as per my comment above.


Cheers,
Gustavo