On 03/03/26, Helge Deller wrote: > On 3/2/26 12:56, Anton Johansson wrote: > > Signed-off-by: Anton Johansson <[email protected]> > > --- > > target/hppa/cpu.h | 11 ++++++++--- > > hw/hppa/machine.c | 4 ++-- > > hw/pci-host/astro.c | 2 +- > > target/hppa/mem_helper.c | 40 ++++++++++++---------------------------- > > 4 files changed, 23 insertions(+), 34 deletions(-) > > > > ... > > diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c > > index 9199d1e06a..4bd806a53f 100644 > > --- a/target/hppa/mem_helper.c > > +++ b/target/hppa/mem_helper.c > > @@ -29,29 +29,12 @@ > > #include "hw/core/cpu.h" > > #include "trace.h" > > -/* > > - * 64-bit (PA-RISC 2.0) machines are assumed to run PA-8700, and 32-bit > > - * machines 7300LC. This should give 44 and 32 bits of physical address > > - * space respectively. > > - * > > - * CPU model Physical address space bits > > - * PA-7000--7300LC 32 > > - * PA-8000--8600 40 > > - * PA-8700--8900 44 > > - * > > - * FIXME: However, the SeaBIOS firmware that is that tested against > > - * uses 40-bit physical addresses, despite supposedly running a C3700 > > - * with a PA-8700 cpu, so use 40-bits for 64-bit. > > - */ > > -#define HPPA_PHYS_ADDR_SPACE_BITS_PA20 40 > > -#define HPPA_PHYS_ADDR_SPACE_BITS_PA1X 32 > > - > > -hwaddr hppa_abs_to_phys_pa1x(vaddr addr) > > +hwaddr hppa_abs_to_phys_pa1x(CPUHPPAState *env, vaddr addr) > > { > > - return extract64(addr, 0, HPPA_PHYS_ADDR_SPACE_BITS_PA1X); > > + return extract64(addr, 0, hppa_phys_addr_bits(env)); > > } > > -hwaddr hppa_abs_to_phys_pa2_w1(vaddr addr) > > +hwaddr hppa_abs_to_phys_pa2_w1(CPUHPPAState *env, vaddr addr) > > { > > /* > > * Figure H-8 "62-bit Absolute Accesses when PSW W-bit is 1" describes > > @@ -64,11 +47,12 @@ hwaddr hppa_abs_to_phys_pa2_w1(vaddr addr) > > * Since the supported physical address space is below 54 bits, the > > * H-8 algorithm is moot and all that is left is to truncate. > > */ > > - QEMU_BUILD_BUG_ON(HPPA_PHYS_ADDR_SPACE_BITS_PA20 > 54); > > - return sextract64(addr, 0, HPPA_PHYS_ADDR_SPACE_BITS_PA20); > > + const uint8_t pa = hppa_phys_addr_bits(env); > > + g_assert(pa <= 54); > > This g_assert() is now a runtime check which adds cycles for every > hppa_abs_to_phys_pa2_w1() call, > while the previous QEMU_BUILD_BUG_ON() was a cheap compile-time check. > Can we change it, or simply drop it, because hppa_phys_addr_bits() anyways > return a constant?
Good point, I think having the sanity check somewhere still makes sense since we are assuming that it's less than 54. I'll move it to the initialization of the CPU base class, so it's only ran once. > Other than that: > Reviewed-by: Helge Deller <[email protected]> Thanks!
