On 30/4/25 08:26, Philippe Mathieu-Daudé wrote:
Hi,
On 13/2/25 13:37, Philippe Mathieu-Daudé wrote:
+AMD folks
On 12/2/25 23:01, Richard Henderson wrote:
Use out-of-line helpers to implement extended address memory ops.
With this, we can reduce TARGET_LONG_BITS to the more natural 32
for this 32-bit cpu.
I thought about something similar 2 months ago, but then realized
MicroBlaze cores can be synthetized in 64-bit, and IIRC there is
not much missing (I'd say effort would be to add 20% more of what
we currently have). Just wanted to mention before taking the
decision to restrict to 32-bit. OTOH if there are no plan for
adding 64-bit support at AMD, then I'm more than happy to simplify
by considering only 32-bit.
I gave this series another go, and figured the microblaze target
addition was done way before the 64-bit. C_DATA_SIZE value was fixed
as 32, and C_ADDR_SIZE was not mentioned. Later C_DATA_SIZE became
configurable as [32, 64] and C_ADDR_SIZE appeared.
FTR C_ADDR_SIZE starts to be mentioned in Vivado 2016.1 release as
• Included description of address extension, new in version 9.6.
Commit 72e387548534 (Jun 18 2015) made explicit supported versions
were 5.00.a up to 9.3 (per Vivado 2014.1 release).
Commit d79fcbc298b0 (Jan 11 2017) "Add CPU versions 9.4, 9.5 and 9.6",
and commit feac83af3be6 (Jun 15 2017) "Add CPU version 10.0" (released
in Vivado 2016.3, but MMU Physical Address Extension 'PAE' came in
Vivado 2017.1).
Vivado 2018.3 added MicroBlaze 64-bit implementation "new in version 11.0".
IIUC current implementation is correct w.r.t. v9.5.
I'm not so sure we can announce v9.6 and v10.0 as correctly implemented.
Looking at what our machines uses, latest is v8.40.b:
hw/microblaze/petalogix_ml605_mmu.c:88:
object_property_set_str(OBJECT(cpu), "version", "8.10.a", &error_abort);
hw/microblaze/petalogix_s3adsp1800_mmu.c:78:
object_property_set_str(OBJECT(cpu), "version", "7.10.d", &error_abort);
hw/microblaze/xlnx-zynqmp-pmu.c:95:
object_property_set_str(OBJECT(&s->cpu), "version", "8.40.b",
Maybe we can deprecate / remove v9.6 & v10.0 to better add them with
a proper microblaze64 target implementation?
Indeed what this series does is correctly implement the current
target as C_DATA_SIZE=32 (C_ADDR_SIZE=32 implied).
I had a quick look at what is missing for C_DATA_SIZE > 32 and it
is more than the 20% I first roughly estimated. So with the current
implementation, this series is doing the right thing IMHO.
Regards,
Phil.