On 2025/10/18 上午2:47, Richard Henderson wrote:
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.
It is to convert it to physical address for compatible issue. With page
directory table, HW discards bits higher than
TARGET_PHYS_ADDR_SPACE_BITS for compatible issue since SW has already
used in this way.
SW sets higher bit and treats it as virtual address, software can use it
directly with set_pXd() and needs not convert to physical address. In
future SW can use page directory table with physical address method,
however there is no obvious benefits and motivation :(
Regards
Bibo Mao
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~
