Re: [PATCH 4/5] target-info: Introduce runtime TARGET_PHYS_ADDR_SPACE_BITS

[Richard Henderson <richard.henderson@linaro.org>]

On 10/17/25 09:11, Anton Johansson wrote:
> Hmm you're right looking at git grep -C1 TARGET_PHYS_ADDR_SPACE_BITS
> (output below excluding the hw/riscv change in the following patch),
> there are really aren't that many uses left and none in common code.
> 
> We still got to move it to a runtime value somewhere though, what
> would be a more suitable location?  Maybe as a field in CPUArchState or
> some parent QOM machine as only i386, hppa, loongarch, riscv, alpha
> actually use the definition.

A fair few of these are arguably wrong.


>    hw/loongarch/boot.c:    return addr & MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS);
>    --
>    hw/loongarch/boot.c-    *kernel_entry = extract64(le64_to_cpu(hdr->kernel_entry),
>    hw/loongarch/boot.c:                              0, TARGET_PHYS_ADDR_SPACE_BITS);
>    hw/loongarch/boot.c-    *kernel_low = extract64(le64_to_cpu(hdr->load_offset),
>    hw/loongarch/boot.c:                            0, TARGET_PHYS_ADDR_SPACE_BITS);

This is cpu_loongarch_virt_to_phys, and some repetitions.

This should probably use a loongarch-specific runtime function to find the address space 
range supported by the chosen cpu.  Or perhaps just a target-specific constant mask.


>    linux-user/alpha/target_proc.h-            "L3 cache\t\t: n/a\n",
>    linux-user/alpha/target_proc.h:            model, TARGET_PAGE_SIZE, TARGET_PHYS_ADDR_SPACE_BITS,
>    linux-user/alpha/target_proc.h-            max_cpus, num_cpus, cpu_mask);

This is the alpha-linux-user implementation of /proc/cpuinfo.

Ideally this should be a target-specific function; see

/* ??? EV4 has 34 phys addr bits, EV5 has 40, EV6 has 44.  */
#define TARGET_PHYS_ADDR_SPACE_BITS  44

It's certainly not generic, and it's also not really important.

>    --
>    target/hppa/mem_helper.c:    QEMU_BUILD_BUG_ON(TARGET_PHYS_ADDR_SPACE_BITS > 54);
>    target/hppa/mem_helper.c:    return sextract64(addr, 0, TARGET_PHYS_ADDR_SPACE_BITS);
>    --
>    target/hppa/mem_helper.c:        addr |= -1ull << (TARGET_PHYS_ADDR_SPACE_BITS - 4);
>    --
>    target/hppa/mem_helper.c-    /* Ignore the bits beyond physical address space. */
>    target/hppa/mem_helper.c:    ent->pa = sextract64(ent->pa, 0, TARGET_PHYS_ADDR_SPACE_BITS);

Similarly

/* ??? PA-8000 through 8600 have 40 bits; PA-8700 and 8900 have 44 bits. */
# define TARGET_PHYS_ADDR_SPACE_BITS  40

While we don't actually name concrete cpu models, bios advertises the (32-bit) HP B160L 
machine, which originally had a 7300LC, and the (64-bit) which had a 8700.

I can't find definitive documentation, but I suspect the 7300LC has only 32 physical 
address bits.  And according to our own comment we get the 8700 value wrong.

In either case, it's not exposed to generic code.

>    --
>    target/i386/cpu.c-        if (cpu->phys_bits &&
>    target/i386/cpu.c:            (cpu->phys_bits > TARGET_PHYS_ADDR_SPACE_BITS ||
>    target/i386/cpu.c-            cpu->phys_bits < 32)) {
>    --
>    target/i386/cpu.c-                             " (but is %u)",
>    target/i386/cpu.c:                             TARGET_PHYS_ADDR_SPACE_BITS, cpu->phys_bits);
>    --
>    target/i386/kvm/kvm.c:        QEMU_BUILD_BUG_ON(TARGET_PHYS_ADDR_SPACE_BITS > 52);
>    target/i386/kvm/kvm.c:        assert(cpu->phys_bits <= TARGET_PHYS_ADDR_SPACE_BITS);

All of these are simply making sure that cpu->phys_bits is "in range", which is now 
irrelevant because TARGET_PHYS_ADDR_SPACE_BITS itself is no longer in use.  They can all 
be removed.

>    --
>    target/i386/tcg/helper-tcg.h:QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS > TARGET_PHYS_ADDR_SPACE_BITS);

Likewise.


>    target/loongarch/internals.h:#define TARGET_PHYS_MASK MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS)

This is used by target/loongarch/tcg/tlb_helper.c.

I'm not sure what the implications are.
Should it be using a common function with the loongarch boot virt-to-phys?
Is it re-using TARGET_PHYS_ADDR_SPACE_BITS just because it was convienient?

In either case, it's not exposed to generic code.


r~