Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[BALATON Zoltan]

On Sun, 2 Mar 2025, Bernhard Beschow wrote:

Am 1. März 2025 16:10:35 UTC schrieb BALATON Zoltan :

On Wed, 15 Jan 2025, BALATON Zoltan wrote:

This allows guests to set the CCSR base address. Also store and return
values of the local access window registers but their functionality
isn't implemented.


Bernhard,


Hi Zoltan,



If you have no alternative patch you plan to submit for next release 
should we merge this for now? This helps running u-boot binaries even 
if it's not enough alone but would be one patch less in my local tree. 
Or do you know about a problem with this patch why this should not be 
merged?


QEMU sets a machine-specific CCSR base address (pmc->ccsrbar_base) which 
differs from the real chip's default. The default is checked by U-Boot 
which enters an infinite loop on mismatch: 
. 
How does this work for you?


The U-Boot version I've tested (or something else, I don't remember) 
wanted to set the CCSRBAR to the value it expects which is different from 
where it looks for it at reset and that works with this patch. I don't 
know which code path that corresponds to in start.S.


In addition, when moving the CCSR region, `env->mpic_iack` should be 
updated as well: 



I did not notice that. Maybe it's only relevant with KVM and I've only 
tested with TCG or could be I did not get to the part yet when this would 
cause problems. I don't see an easy way to update this as these are in 
separate places and also don't know how this mpic_iack is handled with 
multiple cores. So in case this patch breaks this then it's OK to drop it 
for now. I can carry it locally until fixed upstream.


I'm happy to submit an implementation on top of my e500 cleanup series 
 
which needs agreement on some open discussions.


Some of that series was already merged. What are the outstanding patches 
and discussions? I don't remember seeing an updated version of that series 
with only the outstanding patches.


Regards,
BALATON Zoltan

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[Bernhard Beschow]

Am 1. März 2025 16:10:35 UTC schrieb BALATON Zoltan :
>On Wed, 15 Jan 2025, BALATON Zoltan wrote:
>> This allows guests to set the CCSR base address. Also store and return
>> values of the local access window registers but their functionality
>> isn't implemented.
>
>Bernhard,

Hi Zoltan,

>
>If you have no alternative patch you plan to submit for next release should we 
>merge this for now? This helps running u-boot binaries even if it's not enough 
>alone but would be one patch less in my local tree. Or do you know about a 
>problem with this patch why this should not be merged?

QEMU sets a machine-specific CCSR base address (pmc->ccsrbar_base) which 
differs from the real chip's default. The default is checked by U-Boot which 
enters an infinite loop on mismatch: 
.
 How does this work for you?

In addition, when moving the CCSR region, `env->mpic_iack` should be updated as 
well: 


I'm happy to submit an implementation on top of my e500 cleanup series 
 which 
needs agreement on some open discussions.

Best regards,
Bernhard

>
>Regards,
>BALATON Zoltan
>
>> Signed-off-by: BALATON Zoltan 
>> ---
>> hw/ppc/e500-ccsr.h |  4 +++
>> hw/ppc/e500.c  | 79 --
>> 2 files changed, 80 insertions(+), 3 deletions(-)
>> 
>> diff --git a/hw/ppc/e500-ccsr.h b/hw/ppc/e500-ccsr.h
>> index 249c17be3b..cfbf96e181 100644
>> --- a/hw/ppc/e500-ccsr.h
>> +++ b/hw/ppc/e500-ccsr.h
>> @@ -4,12 +4,16 @@
>> #include "hw/sysbus.h"
>> #include "qom/object.h"
>> 
>> +#define NR_LAWS 12
>> +
>> struct PPCE500CCSRState {
>> /*< private >*/
>> SysBusDevice parent;
>> /*< public >*/
>> 
>> MemoryRegion ccsr_space;
>> +
>> +uint32_t law_regs[NR_LAWS * 2];
>> };
>> 
>> #define TYPE_CCSR "e500-ccsr"
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 64f8c766b4..376cb4cb37 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -43,6 +43,7 @@
>> #include "qemu/host-utils.h"
>> #include "qemu/option.h"
>> #include "hw/pci-host/ppce500.h"
>> +#include "qemu/log.h"
>> #include "qemu/error-report.h"
>> #include "hw/platform-bus.h"
>> #include "hw/net/fsl_etsec/etsec.h"
>> @@ -1331,11 +1332,83 @@ void ppce500_init(MachineState *machine)
>> boot_info->dt_size = dt_size;
>> }
>> 
>> +static int law_idx(hwaddr addr)
>> +{
>> +int idx;
>> +
>> +addr -= 0xc08;
>> +idx = 2 * ((addr >> 5) & 0xf);
>> +if (addr & 8) {
>> +idx++;
>> +}
>> +assert(idx < 2 * NR_LAWS);
>> +return idx;
>> +}
>> +
>> +static uint64_t law_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +PPCE500CCSRState *s = opaque;
>> +uint64_t val = 0;
>> +
>> +switch (addr) {
>> +case 0:
>> +val = s->ccsr_space.addr >> 12;
>> +break;
>> +case 0xc08 ... 0xd70:
>> +val = s->law_regs[law_idx(addr)];
>> +break;
>> +default:
>> +qemu_log_mask(LOG_GUEST_ERROR, "Invalid local access register read"
>> +  "0x%" HWADDR_PRIx "\n", addr);
>> +}
>> +return val;
>> +}
>> +
>> +static void law_write(void *opaque, hwaddr addr, uint64_t val, unsigned 
>> size)
>> +{
>> +PPCE500CCSRState *s = opaque;
>> +
>> +switch (addr) {
>> +case 0:
>> +val &= 0x00;
>> +memory_region_set_address(&s->ccsr_space, val << 12);
>> +break;
>> +case 0xc08 ... 0xd70:
>> +{
>> +int idx = law_idx(addr);
>> +
>> +qemu_log_mask(LOG_UNIMP, "Unimplemented local access register write"
>> +  "0x%" HWADDR_PRIx " <- 0x%" PRIx64 "\n", addr, val);
>> +val &= (idx & 1) ? 0x80f0003f : 0xff;
>> +s->law_regs[idx] = val;
>> +break;
>> +}
>> +default:
>> +qemu_log_mask(LOG_GUEST_ERROR, "Invalid local access register write"
>> +  "0x%" HWADDR_PRIx "\n", addr);
>> +}
>> +}
>> +
>> +static const MemoryRegionOps law_ops = {
>> +.read = law_read,
>> +.write = law_write,
>> +.endianness = DEVICE_BIG_ENDIAN,
>> +.valid = {
>> +.min_access_size = 4,
>> +.max_access_size = 4,
>> +},
>> +};
>> +
>> static void e500_ccsr_initfn(Object *obj)
>> {
>> -PPCE500CCSRState *ccsr = CCSR(obj);
>> -memory_region_init(&ccsr->ccsr_space, obj, "e500-ccsr",
>> -   MPC8544_CCSRBAR_SIZE);
>> +PPCE500CCSRState *s = CCSR(obj);
>> +MemoryRegion *mr;
>> +
>> +memory_region_init(&s->ccsr_space, obj, "e500-ccsr", 
>> MPC8544_CCSRBAR_SIZE);
>> +
>> +mr = g_new(MemoryRegion, 1);
>> +memory_region_init_io(mr, obj, &law_ops, s, "local-access", 4096);
>> +memory_region_add_subregion(&s->ccsr_space, 0, mr);
>> }
>> 
>> static const TypeInfo e500_ccsr_info = {
>> 

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[BALATON Zoltan]

On Wed, 15 Jan 2025, BALATON Zoltan wrote:

This allows guests to set the CCSR base address. Also store and return
values of the local access window registers but their functionality
isn't implemented.


Bernhard,

If you have no alternative patch you plan to submit for next release 
should we merge this for now? This helps running u-boot binaries even if 
it's not enough alone but would be one patch less in my local tree. Or do 
you know about a problem with this patch why this should not be merged?


Regards,
BALATON Zoltan


Signed-off-by: BALATON Zoltan 
---
hw/ppc/e500-ccsr.h |  4 +++
hw/ppc/e500.c  | 79 --
2 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/e500-ccsr.h b/hw/ppc/e500-ccsr.h
index 249c17be3b..cfbf96e181 100644
--- a/hw/ppc/e500-ccsr.h
+++ b/hw/ppc/e500-ccsr.h
@@ -4,12 +4,16 @@
#include "hw/sysbus.h"
#include "qom/object.h"

+#define NR_LAWS 12
+
struct PPCE500CCSRState {
/*< private >*/
SysBusDevice parent;
/*< public >*/

MemoryRegion ccsr_space;
+
+uint32_t law_regs[NR_LAWS * 2];
};

#define TYPE_CCSR "e500-ccsr"
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 64f8c766b4..376cb4cb37 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -43,6 +43,7 @@
#include "qemu/host-utils.h"
#include "qemu/option.h"
#include "hw/pci-host/ppce500.h"
+#include "qemu/log.h"
#include "qemu/error-report.h"
#include "hw/platform-bus.h"
#include "hw/net/fsl_etsec/etsec.h"
@@ -1331,11 +1332,83 @@ void ppce500_init(MachineState *machine)
boot_info->dt_size = dt_size;
}

+static int law_idx(hwaddr addr)
+{
+int idx;
+
+addr -= 0xc08;
+idx = 2 * ((addr >> 5) & 0xf);
+if (addr & 8) {
+idx++;
+}
+assert(idx < 2 * NR_LAWS);
+return idx;
+}
+
+static uint64_t law_read(void *opaque, hwaddr addr, unsigned size)
+{
+PPCE500CCSRState *s = opaque;
+uint64_t val = 0;
+
+switch (addr) {
+case 0:
+val = s->ccsr_space.addr >> 12;
+break;
+case 0xc08 ... 0xd70:
+val = s->law_regs[law_idx(addr)];
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "Invalid local access register read"
+  "0x%" HWADDR_PRIx "\n", addr);
+}
+return val;
+}
+
+static void law_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+PPCE500CCSRState *s = opaque;
+
+switch (addr) {
+case 0:
+val &= 0x00;
+memory_region_set_address(&s->ccsr_space, val << 12);
+break;
+case 0xc08 ... 0xd70:
+{
+int idx = law_idx(addr);
+
+qemu_log_mask(LOG_UNIMP, "Unimplemented local access register write"
+  "0x%" HWADDR_PRIx " <- 0x%" PRIx64 "\n", addr, val);
+val &= (idx & 1) ? 0x80f0003f : 0xff;
+s->law_regs[idx] = val;
+break;
+}
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "Invalid local access register write"
+  "0x%" HWADDR_PRIx "\n", addr);
+}
+}
+
+static const MemoryRegionOps law_ops = {
+.read = law_read,
+.write = law_write,
+.endianness = DEVICE_BIG_ENDIAN,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+};
+
static void e500_ccsr_initfn(Object *obj)
{
-PPCE500CCSRState *ccsr = CCSR(obj);
-memory_region_init(&ccsr->ccsr_space, obj, "e500-ccsr",
-   MPC8544_CCSRBAR_SIZE);
+PPCE500CCSRState *s = CCSR(obj);
+MemoryRegion *mr;
+
+memory_region_init(&s->ccsr_space, obj, "e500-ccsr", MPC8544_CCSRBAR_SIZE);
+
+mr = g_new(MemoryRegion, 1);
+memory_region_init_io(mr, obj, &law_ops, s, "local-access", 4096);
+memory_region_add_subregion(&s->ccsr_space, 0, mr);
}

static const TypeInfo e500_ccsr_info = {

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[BALATON Zoltan]

On Thu, 20 Feb 2025, Bernhard Beschow wrote:

Am 13. Februar 2025 00:13:24 UTC schrieb BALATON Zoltan :
Yes, your DTB based board code is a nice way to create different 
machines as the DTB already describes these offsets and irq connections 
and your code seems to be quite simple so I think it's a good idea 
that's worth pursuing, that could enhance the ppce500 machine and make 
it more generic. It could then also drop the separate mpc85xx machine 
and put all of these under one ppce500 machine with passing the right 
dtb to create the different machines. As long as these are similar 
enough and only differ in the devices they have, this could emulate a 
lot of these SoCs with the same code.


The existing machines can already be created from a DTB, see the pc-bios 
folder which contains the two respective .dts files.


That's why I said that once this is merged we could deprecate the existing 
machines in favour of ppc500 with the dts for these machines replacing 
them so we can get rid of board codes and can add new machines like p1022 
by adding the needed devices and the dts also under ppce500 which can 
become a generic machine supporting all of these. One could then use -M 
ppce500 -dtb mpc85xx.dtb or p1022.dtb or similar. This would work for all 
dtbs that we have devices for and maybe by adding unimplemented device for 
unknown ones would allow somewhat booting other machines as long as those 
devices are not use. Or if keeping a machine for the -M option for all of 
these is desired then we can think of adding some other kind of alias that 
can set the dtb for the ppce500 and call that. But in any case this would 
reduce the mpc85xx to a trivial wrapper of ppce500 that just sets the dtb 
for the machine.


Thanks for your input, I'll look into it closer after my RFC. Right now 
I'm quite busy driving the i.MX 8M Plus machine forward, hence my 
delayed responses.


What I may contribute is new device emulation for missing parts. I have 
some dummy sata that does nothing but allows the Linux driver to pass 
detecting no devices, a half working DIU I made in the last few days 
that needs more work but I got some image from U-Boot on it and may 
look at the DMA controller in the future. Let me know if you're 
interested in these or have something for these or other parts I could 
use instead. I've tested your SPI flash implementation but it wasn't 
working with U-Boot for me and may look at your USB eventually.


Your additions sound promising! And thanks for testing the devices.


I also got distracted with other machines so I've postponed this for now 
but want to get back to it eventually.


Regards,
BALATON Zoltan

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[Bernhard Beschow]

Am 13. Februar 2025 00:13:24 UTC schrieb BALATON Zoltan :
>On Wed, 12 Feb 2025, Bernhard Beschow wrote:
>> Am 7. Februar 2025 01:12:38 UTC schrieb BALATON Zoltan :
>>> On Thu, 6 Feb 2025, Bernhard Beschow wrote:
 Am 2. Februar 2025 01:25:22 UTC schrieb BALATON Zoltan 
 :
 
 I had to apply 
 
  to have the SPL load the whole U-Boot proper.
>>> 
>>> Is that an alternative to commenting out page_aligned = true?
>> 
>> Well, it's not needed with the patch applied. The patch ensures that all 
>> data gets loaded: 
>> 
>
>I think the block layer has a solution for such long running operations and 
>uses coroutines for that but I don't know how that works and I could not find 
>useful documentation on it. But I don't understand the problem either so if 
>you've solved it and submit a patch that can be merged that's good enough for 
>me.
>
>>> There are two U-Boot binaries on the card for some reason (I think maybe 
>>> the first one runs from cache as RAM and sets up the memory controller), 
>>> then the first one loads some env variables and then the second U-Boot 
>>> which then loads the bootloader.
>> 
>> Yeah, that's the SPL and U-Boot proper. The first one sets up RAM based on 
>> DDR3 data, copies U-Boot proper there (currently broken in QEMU, see above), 
>> and passes control to it.
>
>U-Boot proper runs for me too but it may have a bug in the Tabor specific 
>patch (maybe revealed by missing emulation) that prevents the bootloader to 
>find a device but I can run until U-Boot and enter commands to it, it only 
>stops in the bootloader when that calls back to U-Boot. If you extract U-Boot 
>proper from the SD card image, convert it to elf and run it with -bios you'd 
>get a prompt so it works. That's what I do now for experimenting but I'd like 
>it to boot from the SD card the same way as real machine does eventually.
>
>> Considering the technical manual it may be possible to compile U-Boot 
>> oneself and replace components of the firmware image. That way, one could 
>> track down why no bootable devices are found, i.e. check whether and which 
>> bootmenu_x variables are populated. That's not on my todo list though.
>
>First of all one would need the source for that which should be available 
>because of GPL but it's not, at least I could not find it yet. Eventually I'll 
>get it and may look at this but I'd also like to run binary known to work on 
>real machine to make sure emulation is correct.
>
>>> These existing machines set up values in PPCE500MachineClass in their init 
>>> methods that the e500.c uses to change its behaviour to match the machine 
>>> so to continue adding another board in the classic way I'd continue like 
>>> that. I've added another similar board file like those machines setting the 
>>> values matching P1022. For the additional devices in e500.c I've just 
>>> patched them in for experimenting but these could be optionally created 
>>> based on new values in the MachineClass, like has_2nd_i2c or similar to not 
>>> change existing machines.
>> 
>> The challenge is that different variants of the SoC have the same IP cores 
>> mapped at different offsets with possibly (haven't checked) IRQs. These need 
>> to be considered when generating the DTB. To avoid dealing with this 
>> question -- and at the same time explore data-driven machine creation -- I 
>> reversed the process and generated the machine from the DTB. But this 
>> question needs to be answered when implementing a P1022.
>
>Yes, your DTB based board code is a nice way to create different machines as 
>the DTB already describes these offsets and irq connections and your code 
>seems to be quite simple so I think it's a good idea that's worth pursuing, 
>that could enhance the ppce500 machine and make it more generic. It could then 
>also drop the separate mpc85xx machine and put all of these under one ppce500 
>machine with passing the right dtb to create the different machines. As long 
>as these are similar enough and only differ in the devices they have, this 
>could emulate a lot of these SoCs with the same code.

The existing machines can already be created from a DTB, see the pc-bios folder 
which contains the two respective .dts files.

>
>>> I would not go into more elaborate solutions if your fdt based machine 
>>> creation replaces this eventually.
>> 
>> As said before, I'd send an RFC for discussion on how to proceed.
>
>As I wrote before I think one issue to solve as a next step would be avoid 
>needing subclasses with comma in names for all compatible devices. I think 
>allowing alternative names for the same type in the types hash table could be 
>enough but to hide these from users and -device which can't take comma names, 
>these might need to go in a separate hash table instead. This could work the 
>same 

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[BALATON Zoltan]

On Wed, 12 Feb 2025, Bernhard Beschow wrote:

Am 7. Februar 2025 01:12:38 UTC schrieb BALATON Zoltan :

On Thu, 6 Feb 2025, Bernhard Beschow wrote:

Am 2. Februar 2025 01:25:22 UTC schrieb BALATON Zoltan :

I had to apply 
 
to have the SPL load the whole U-Boot proper.


Is that an alternative to commenting out page_aligned = true?


Well, it's not needed with the patch applied. The patch ensures that all 
data gets loaded: 



I think the block layer has a solution for such long running operations 
and uses coroutines for that but I don't know how that works and I could 
not find useful documentation on it. But I don't understand the problem 
either so if you've solved it and submit a patch that can be merged that's 
good enough for me.


There are two U-Boot binaries on the card for some reason (I think 
maybe the first one runs from cache as RAM and sets up the memory 
controller), then the first one loads some env variables and then the 
second U-Boot which then loads the bootloader.


Yeah, that's the SPL and U-Boot proper. The first one sets up RAM based 
on DDR3 data, copies U-Boot proper there (currently broken in QEMU, see 
above), and passes control to it.


U-Boot proper runs for me too but it may have a bug in the Tabor specific 
patch (maybe revealed by missing emulation) that prevents the bootloader 
to find a device but I can run until U-Boot and enter commands to it, it 
only stops in the bootloader when that calls back to U-Boot. If you 
extract U-Boot proper from the SD card image, convert it to elf and run it 
with -bios you'd get a prompt so it works. That's what I do now for 
experimenting but I'd like it to boot from the SD card the same way as 
real machine does eventually.


Considering the technical manual it may be possible to compile U-Boot 
oneself and replace components of the firmware image. That way, one 
could track down why no bootable devices are found, i.e. check whether 
and which bootmenu_x variables are populated. That's not on my todo list 
though.


First of all one would need the source for that which should be available 
because of GPL but it's not, at least I could not find it yet. Eventually 
I'll get it and may look at this but I'd also like to run binary known to 
work on real machine to make sure emulation is correct.


These existing machines set up values in PPCE500MachineClass in their 
init methods that the e500.c uses to change its behaviour to match the 
machine so to continue adding another board in the classic way I'd 
continue like that. I've added another similar board file like those 
machines setting the values matching P1022. For the additional devices 
in e500.c I've just patched them in for experimenting but these could 
be optionally created based on new values in the MachineClass, like 
has_2nd_i2c or similar to not change existing machines.


The challenge is that different variants of the SoC have the same IP 
cores mapped at different offsets with possibly (haven't checked) IRQs. 
These need to be considered when generating the DTB. To avoid dealing 
with this question -- and at the same time explore data-driven machine 
creation -- I reversed the process and generated the machine from the 
DTB. But this question needs to be answered when implementing a P1022.


Yes, your DTB based board code is a nice way to create different machines 
as the DTB already describes these offsets and irq connections and your 
code seems to be quite simple so I think it's a good idea that's worth 
pursuing, that could enhance the ppce500 machine and make it more generic. 
It could then also drop the separate mpc85xx machine and put all of these 
under one ppce500 machine with passing the right dtb to create the 
different machines. As long as these are similar enough and only differ in 
the devices they have, this could emulate a lot of these SoCs with the 
same code.


I would not go into more elaborate solutions if your fdt based machine 
creation replaces this eventually.


As said before, I'd send an RFC for discussion on how to proceed.


As I wrote before I think one issue to solve as a next step would be avoid 
needing subclasses with comma in names for all compatible devices. I think 
allowing alternative names for the same type in the types hash table could 
be enough but to hide these from users and -device which can't take comma 
names, these might need to go in a separate hash table instead. This could 
work the same as register_type but would be something like 
register_compatible_name_for_type that adds only a name for an existing 
type so it does not need a subclass and duplicate class or type struct 
when all we need is a different name for the existing type. I don't know 
if it's best put in a new hash table or the same one that holds type 
names. E.g. machine names already have 

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[Bernhard Beschow]

Am 7. Februar 2025 01:12:38 UTC schrieb BALATON Zoltan :
>On Thu, 6 Feb 2025, Bernhard Beschow wrote:
>> Am 2. Februar 2025 01:25:22 UTC schrieb BALATON Zoltan :
>>> On Sat, 1 Feb 2025, Bernhard Beschow wrote:
 Am 1. Februar 2025 14:55:15 UTC schrieb Bernhard Beschow 
 :
> Am 30. Januar 2025 12:45:58 UTC schrieb BALATON Zoltan 
> :
>> On Wed, 15 Jan 2025, BALATON Zoltan wrote:
>>> This allows guests to set the CCSR base address. Also store and return
>>> values of the local access window registers but their functionality
>>> isn't implemented.
>> 
>> Ping?
> 
> I guess you're trying to boot a real firmware image from SD card?
>>> 
>>> I'm not trying, I've done it. I only needed these patches and commenting 
>>> out the page_aligned = true in hw/sd/sdhci.c as I noted in the previous 
>>> patch.
>> 
>> I had to apply 
>> 
>>  to have the SPL load the whole U-Boot proper.
>
>Is that an alternative to commenting out page_aligned = true?

Well, it's not needed with the patch applied. The patch ensures that all data 
gets loaded: 


>(Previously it also needed some tweak on when to set DMA bit but after your 
>fix in commit 5df50b8e973 in master resolved it that's no longer needed. Now 
>only the interrupt control reset values and commenting page_aligned = true was 
>needed to get it work.)
>
>>> U-Boot works and I can run commands in the firmware prompt but I did not 
>>> try to boot an OS yet. The amigaos boot loader which U-Boot for Tabor loads 
>>> by default crashes somewhere but this may be a bug in the patched U-Boot. I 
>>> think I've seen similar with sam460ex U-Boot before, maybe it's because of 
>>> not finding some expected devices and not handling the returned NULL value 
>>> well but I did not debug it.
>> 
>> Do you use the Tabor U-Boot or something else?
>
>I've tested the firmware image available from this page:
>http://eliyahu.org/tabor/setup.html
>There's also a technical manual there that has info on the content of the SD 
>card image.

I got both from there as well.

>
>> How do you get to the command prompt? For me, the bootloader application 
>> started by Tabor U-Boot doesn't crash but then doesn't find bootable 
>> devices, not even with an emulated USB stick. Instead of dropping to the 
>> command prompt it only offers a restart to the firmware which then starts 
>> the bootloader application again...
>
>There are two U-Boot binaries on the card for some reason (I think maybe the 
>first one runs from cache as RAM and sets up the memory controller), then the 
>first one loads some env variables and then the second U-Boot which then loads 
>the bootloader.

Yeah, that's the SPL and U-Boot proper. The first one sets up RAM based on DDR3 
data, copies U-Boot proper there (currently broken in QEMU, see above), and 
passes control to it.

>If you change one byte in the environment on the SD card it breaks the 
>checksum and then it does not load the bootloader but gives a prompt. You can 
>then look around in U-Boot. Alternatively you can extract the U-Boot binaries 
>and convert to elf to load it with -bios then you can skip the SD card. Maybe 
>depending on some env settings I haven't identified yet, the bootloader either 
>says no bootable devices found or crashes when calling back into U-Boot which 
>may be something similar that I had to fix for sam460ex here: 
>https://gitlab.com/qemu-project/u-boot-sam460ex/-/commit/f6402c160f781145206b2dc0eb4d50738d0531d4/
> but I don't have the Tabor U-Boot sources yet to check. Presumably it works 
>on real machine so maybe it checks for SATA or other devices which aren't 
>emulated so it may get an unexpected NULL value. I've tried adding dummy SATA 
>emulation that shows an empty bus but it did not fix that. It could also be 
>something completely different. With the USB patch, it at least finds USB 
>storage devices after usb start in U-Boot but that's all I could test. I 
>should find some Linux boot media known to work on real machine to test 
>further but haven't had time for it.

Considering the technical manual it may be possible to compile U-Boot oneself 
and replace components of the firmware image. That way, one could track down 
why no bootable devices are found, i.e. check whether and which bootmenu_x 
variables are populated. That's not on my todo list though.

>
> I've implemented that in my e500-fdt branch which I want to send as an 
> RFC. I still need to clean it up. Once it's on the list we could make a 
> plan how to turn it into a p10xx. Would that work for you?
>>> 
>>> Sure, I can try to test your patches once they are submitted to the list 
>>> and rebase my changes on top if they still needed. I've just submitted 
>>> these so you can incorporate them in your tree so I have less to rebase but 
>>> I 

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[BALATON Zoltan]

On Sun, 2 Feb 2025, BALATON Zoltan wrote:

On Sat, 1 Feb 2025, Bernhard Beschow wrote:
I've implemented that in my e500-fdt branch which I want to send as an 
RFC. I still need to clean it up. Once it's on the list we could make a 
plan how to turn it into a p10xx. Would that work for you?


Sure, I can try to test your patches once they are submitted to the list and 
rebase my changes on top if they still needed. I've just submitted these so 
you can incorporate them in your tree so I have less to rebase but I see you 
already have most of these. I'm OK to wait until your tree is cleaned and 
submitted but it seems there are a lot of patches so it might take a while. I 
don't expect that you can get it merged before the next release. Some of the 
patches may need several versions or alternative approaches until they can be 
merged. For example I expect problems with allowing ',' in device names as 
this is something that was removed before to avoid the need of quoting or 
something like that. But I'm not in a hurry as I don't have much free time 
for it anyway so only come back to this time to time and it's far from 
anything useful yet.


To solve the problem of needing reintroducing ',' in type names and adding 
a new subclass for every compatible device variant (which might also be 
wasteful if this creates a new type struct for these) maybe you'd have to 
attach a string array to classes or type info instead with all the 
compatible names and then look for the type to instantiate based on that. 
I don't know what the best way would be for that, adding a class property 
or adding this array to type? And maybe you need a new funcion to find the 
type instead of qdev_try_new().


I don't know how this works but the types seem to be stored in a hash 
table so instead of an array of compatible names maybe all that's needed 
is to allow adding alternative names for the same type in this type hash 
table or add another hash table for those alternative names then types 
could be looked up the same way from that by the compatible names without 
needing to subclass them multiple times. If these are separated hash 
tables then ',' may be allowed in the compatible names table or if it's 
the same types table then maybe the names can be encoded in some way to 
separate them from normal type names and avoid the need to reintroduce 
','. These are just vague ideas but maybe gives you some inspiration for 
which way to go to get closer to be able to upstream it.


Regards,
BALATON Zoltan

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[BALATON Zoltan]

On Thu, 6 Feb 2025, Bernhard Beschow wrote:

Am 2. Februar 2025 01:25:22 UTC schrieb BALATON Zoltan :

On Sat, 1 Feb 2025, Bernhard Beschow wrote:

Am 1. Februar 2025 14:55:15 UTC schrieb Bernhard Beschow :

Am 30. Januar 2025 12:45:58 UTC schrieb BALATON Zoltan :

On Wed, 15 Jan 2025, BALATON Zoltan wrote:

This allows guests to set the CCSR base address. Also store and return
values of the local access window registers but their functionality
isn't implemented.


Ping?


I guess you're trying to boot a real firmware image from SD card?


I'm not trying, I've done it. I only needed these patches and 
commenting out the page_aligned = true in hw/sd/sdhci.c as I noted in 
the previous patch.


I had to apply 
 
to have the SPL load the whole U-Boot proper.


Is that an alternative to commenting out page_aligned = true? (Previously 
it also needed some tweak on when to set DMA bit but after your fix in 
commit 5df50b8e973 in master resolved it that's no longer needed. Now only 
the interrupt control reset values and commenting page_aligned = true was 
needed to get it work.)


U-Boot works and I can run commands in the firmware prompt but I did 
not try to boot an OS yet. The amigaos boot loader which U-Boot for 
Tabor loads by default crashes somewhere but this may be a bug in the 
patched U-Boot. I think I've seen similar with sam460ex U-Boot before, 
maybe it's because of not finding some expected devices and not 
handling the returned NULL value well but I did not debug it.


Do you use the Tabor U-Boot or something else?


I've tested the firmware image available from this page:
http://eliyahu.org/tabor/setup.html
There's also a technical manual there that has info on the content of the 
SD card image.


How do you get to the command prompt? For me, the bootloader application 
started by Tabor U-Boot doesn't crash but then doesn't find bootable 
devices, not even with an emulated USB stick. Instead of dropping to the 
command prompt it only offers a restart to the firmware which then 
starts the bootloader application again...


There are two U-Boot binaries on the card for some reason (I think maybe 
the first one runs from cache as RAM and sets up the memory controller), 
then the first one loads some env variables and then the second U-Boot 
which then loads the bootloader. If you change one byte in the environment 
on the SD card it breaks the checksum and then it does not load the 
bootloader but gives a prompt. You can then look around in U-Boot. 
Alternatively you can extract the U-Boot binaries and convert to elf to 
load it with -bios then you can skip the SD card. Maybe depending on some 
env settings I haven't identified yet, the bootloader either says no 
bootable devices found or crashes when calling back into U-Boot which may 
be something similar that I had to fix for sam460ex here: 
https://gitlab.com/qemu-project/u-boot-sam460ex/-/commit/f6402c160f781145206b2dc0eb4d50738d0531d4/ 
but I don't have the Tabor U-Boot sources yet to check. Presumably it 
works on real machine so maybe it checks for SATA or other devices which 
aren't emulated so it may get an unexpected NULL value. I've tried adding 
dummy SATA emulation that shows an empty bus but it did not fix that. It 
could also be something completely different. With the USB patch, it at 
least finds USB storage devices after usb start in U-Boot but that's all I 
could test. I should find some Linux boot media known to work on real 
machine to test further but haven't had time for it.


I've implemented that in my e500-fdt branch which I want to send as 
an RFC. I still need to clean it up. Once it's on the list we could 
make a plan how to turn it into a p10xx. Would that work for you?


Sure, I can try to test your patches once they are submitted to the 
list and rebase my changes on top if they still needed. I've just 
submitted these so you can incorporate them in your tree so I have less 
to rebase but I see you already have most of these. I'm OK to wait 
until your tree is cleaned and submitted but it seems there are a lot 
of patches so it might take a while. I don't expect that you can get it 
merged before the next release. Some of the patches may need several 
versions or alternative approaches until they can be merged. For 
example I expect problems with allowing ',' in device names as this is 
something that was removed before to avoid the need of quoting or 
something like that. But I'm not in a hurry as I don't have much free 
time for it anyway so only come back to this time to time and it's far 
from anything useful yet.


My branch is not a maintainer tree. I neither expect it to be mergeable 
like this, nor is it my goal. Instead, it's just an experiment to 
investigate data-driven machine creation which I'd like to share as an 
RFC with the community.


That said, one could probably turn that branch into a p10xx SoC 
implemented in the 

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[Bernhard Beschow]

Am 2. Februar 2025 01:25:22 UTC schrieb BALATON Zoltan :
>On Sat, 1 Feb 2025, Bernhard Beschow wrote:
>> Am 1. Februar 2025 14:55:15 UTC schrieb Bernhard Beschow :
>>> Am 30. Januar 2025 12:45:58 UTC schrieb BALATON Zoltan :
 On Wed, 15 Jan 2025, BALATON Zoltan wrote:
> This allows guests to set the CCSR base address. Also store and return
> values of the local access window registers but their functionality
> isn't implemented.
 
 Ping?
>>> 
>>> I guess you're trying to boot a real firmware image from SD card?
>
>I'm not trying, I've done it. I only needed these patches and commenting out 
>the page_aligned = true in hw/sd/sdhci.c as I noted in the previous patch.

I had to apply 

 to have the SPL load the whole U-Boot proper.

>U-Boot works and I can run commands in the firmware prompt but I did not try 
>to boot an OS yet. The amigaos boot loader which U-Boot for Tabor loads by 
>default crashes somewhere but this may be a bug in the patched U-Boot. I think 
>I've seen similar with sam460ex U-Boot before, maybe it's because of not 
>finding some expected devices and not handling the returned NULL value well 
>but I did not debug it.

Do you use the Tabor U-Boot or something else? How do you get to the command 
prompt? For me, the bootloader application started by Tabor U-Boot doesn't 
crash but then doesn't find bootable devices, not even with an emulated USB 
stick. Instead of dropping to the command prompt it only offers a restart to 
the firmware which then starts the bootloader application again...

>
>>> I've implemented that in my e500-fdt branch which I want to send as an RFC. 
>>> I still need to clean it up. Once it's on the list we could make a plan how 
>>> to turn it into a p10xx. Would that work for you?
>
>Sure, I can try to test your patches once they are submitted to the list and 
>rebase my changes on top if they still needed. I've just submitted these so 
>you can incorporate them in your tree so I have less to rebase but I see you 
>already have most of these. I'm OK to wait until your tree is cleaned and 
>submitted but it seems there are a lot of patches so it might take a while. I 
>don't expect that you can get it merged before the next release. Some of the 
>patches may need several versions or alternative approaches until they can be 
>merged. For example I expect problems with allowing ',' in device names as 
>this is something that was removed before to avoid the need of quoting or 
>something like that. But I'm not in a hurry as I don't have much free time for 
>it anyway so only come back to this time to time and it's far from anything 
>useful yet.

My branch is not a maintainer tree. I neither expect it to be mergeable like 
this, nor is it my goal. Instead, it's just an experiment to investigate 
data-driven machine creation which I'd like to share as an RFC with the 
community.

That said, one could probably turn that branch into a p10xx SoC implemented in 
the classic way. For this, one would need to decide on how to proceed with the 
mpc8544ds and e500plat machines. There are Buildroot recipes for the machines, 
both 32 and 64 bit, which might be nice to keep working -- ideas welcome. Once 
the p10xx SoC is implemented, a tabor machine could be added which uses it.

>
>>> 
>>> Best regards,
>>> Bernhard
>>> 
>>> P.S. The last commit teaches you how to start a firmware from SD card.
>
>I did not try your version but looking at the patch looks like you have some 
>sparse-mem region. (I've added similar one from board code, I did not know 
>about this device.) Isn't that the l2cache as RAM or on-chip SRAM or whatever 
>it's called? You seem to have some emulation of that l2cache in a previous 
>patch so can't that be mapped there? Maybe we'll eventually need to implement 
>reading the BOOT data from the beginning of the SD card or flash ROM which may 
>have some initial register values that set things up that are currently hard 
>coded.

This is implemented on my branch. It pokes the L2 cache registers to configure 
some (but not all) SRAM to load the SPL to. This SPL uses cache as RAM which 
I'm emulating with a modified sparse-mem region device.

>
>Meanwhile I can cherry pick some patches from your tree and test them. Looks 
>like you have some DDR3 support added, I haven't got to that yet.
>
>For USB I had this for now:
>
>diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>index ee17acdb69..54890d25f3 100644
>--- a/hw/ppc/e500.c
>+++ b/hw/ppc/e500.c
>@@ -900,6 +900,29 @@ static void ppce500_power_off(void *opaque, int line, int 
>on)
> }
> }
>
>+static uint64_t usb_read(void *opaque, hwaddr addr, unsigned size)
>+{
>+switch (addr) {
>+case 0:
>+return BIT(2) | BIT(17);
>+}
>+return 0;
>+}
>+
>+static void usb_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>+{
>+}
>+
>+static const MemoryRegionOps usb_ops = {
>+.read = usb

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[BALATON Zoltan]

On Sat, 1 Feb 2025, Bernhard Beschow wrote:

Am 1. Februar 2025 14:55:15 UTC schrieb Bernhard Beschow :

Am 30. Januar 2025 12:45:58 UTC schrieb BALATON Zoltan :

On Wed, 15 Jan 2025, BALATON Zoltan wrote:

This allows guests to set the CCSR base address. Also store and return
values of the local access window registers but their functionality
isn't implemented.


Ping?


I guess you're trying to boot a real firmware image from SD card?


I'm not trying, I've done it. I only needed these patches and commenting 
out the page_aligned = true in hw/sd/sdhci.c as I noted in the previous 
patch. U-Boot works and I can run commands in the firmware prompt but I 
did not try to boot an OS yet. The amigaos boot loader which U-Boot for 
Tabor loads by default crashes somewhere but this may be a bug in the 
patched U-Boot. I think I've seen similar with sam460ex U-Boot before, 
maybe it's because of not finding some expected devices and not handling 
the returned NULL value well but I did not debug it.


I've implemented that in my e500-fdt branch which I want to send as an 
RFC. I still need to clean it up. Once it's on the list we could make a 
plan how to turn it into a p10xx. Would that work for you?


Sure, I can try to test your patches once they are submitted to the list 
and rebase my changes on top if they still needed. I've just submitted 
these so you can incorporate them in your tree so I have less to rebase 
but I see you already have most of these. I'm OK to wait until your tree 
is cleaned and submitted but it seems there are a lot of patches so it 
might take a while. I don't expect that you can get it merged before the 
next release. Some of the patches may need several versions or alternative 
approaches until they can be merged. For example I expect problems with 
allowing ',' in device names as this is something that was removed before 
to avoid the need of quoting or something like that. But I'm not in a 
hurry as I don't have much free time for it anyway so only come back to 
this time to time and it's far from anything useful yet.




Best regards,
Bernhard

P.S. The last commit teaches you how to start a firmware from SD card.


I did not try your version but looking at the patch looks like you have 
some sparse-mem region. (I've added similar one from board code, I did not 
know about this device.) Isn't that the l2cache as RAM or on-chip SRAM or 
whatever it's called? You seem to have some emulation of that l2cache in a 
previous patch so can't that be mapped there? Maybe we'll eventually need 
to implement reading the BOOT data from the beginning of the SD card or 
flash ROM which may have some initial register values that set things up 
that are currently hard coded.


Meanwhile I can cherry pick some patches from your tree and test them. 
Looks like you have some DDR3 support added, I haven't got to that yet.


For USB I had this for now:

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index ee17acdb69..54890d25f3 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -900,6 +900,29 @@ static void ppce500_power_off(void *opaque, int line, int 
on)
 }
 }

+static uint64_t usb_read(void *opaque, hwaddr addr, unsigned size)
+{
+switch (addr) {
+case 0:
+return BIT(2) | BIT(17);
+}
+return 0;
+}
+
+static void usb_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+}
+
+static const MemoryRegionOps usb_ops = {
+.read = usb_read,
+.write = usb_write,
+.endianness = DEVICE_BIG_ENDIAN,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+};
+
 void ppce500_init(MachineState *machine)
 {
 MemoryRegion *address_space_mem = get_system_memory();
@@ -1118,6 +1141,19 @@ void ppce500_init(MachineState *machine)
 sysbus_mmio_get_region(s, 0));
 }

+/* USB */
+dev = qdev_new("tegra2-ehci-usb");
+s = SYS_BUS_DEVICE(dev);
+sysbus_realize_and_unref(s, &error_fatal);
+sysbus_connect_irq(s, 0, qdev_get_gpio_in(mpicdev, 12));
+memory_region_add_subregion(ccsr_addr_space, 0x22000,
+sysbus_mmio_get_region(s, 0));
+{
+MemoryRegion *mr =  g_new(MemoryRegion, 1);
+memory_region_init_io(mr, OBJECT(dev), &usb_ops, s, "fsl-ehci", 4);
+memory_region_add_subregion(ccsr_addr_space, 0x22500, mr);
+}
+
 /* General Utility device */
 dev = qdev_new("mpc8544-guts");
 s = SYS_BUS_DEVICE(dev);

which is reusing a sufficiently similar existing device just to have 
minimal changes. This isn't the right way but since most of these just 
differ in the reg offsets I wonder if we could turn these offsets into 
properties so we don't need to add a new subclass for every device. I 
think subclasses came from the pci version where the PCI IDs are different 
and maybe sysbus was modelled after that but we only need subclasses where 
additional registers are needed (which may be the case for this fsl device 
so this pr

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[Bernhard Beschow]

Am 1. Februar 2025 14:55:15 UTC schrieb Bernhard Beschow :
>
>
>Am 30. Januar 2025 12:45:58 UTC schrieb BALATON Zoltan :
>>On Wed, 15 Jan 2025, BALATON Zoltan wrote:
>>> This allows guests to set the CCSR base address. Also store and return
>>> values of the local access window registers but their functionality
>>> isn't implemented.
>>
>>Ping?
>
>I guess you're trying to boot a real firmware image from SD card? I've 
>implemented that in my e500-fdt branch which I want to send as an RFC. I still 
>need to clean it up. Once it's on the list we could make a plan how to turn it 
>into a p10xx. Would that work for you?
>
>Best regards,
>Bernhard
>
>P.S. The last commit teaches you how to start a firmware from SD card.

Forgot to mention the link to the branch. Here it is: 
https://github.com/shentok/qemu/tree/e500-fdt

Best regards,
Bernhard

>
>>
>>> Signed-off-by: BALATON Zoltan 
>>> ---
>>> hw/ppc/e500-ccsr.h |  4 +++
>>> hw/ppc/e500.c  | 79 --
>>> 2 files changed, 80 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/hw/ppc/e500-ccsr.h b/hw/ppc/e500-ccsr.h
>>> index 249c17be3b..cfbf96e181 100644
>>> --- a/hw/ppc/e500-ccsr.h
>>> +++ b/hw/ppc/e500-ccsr.h
>>> @@ -4,12 +4,16 @@
>>> #include "hw/sysbus.h"
>>> #include "qom/object.h"
>>> 
>>> +#define NR_LAWS 12
>>> +
>>> struct PPCE500CCSRState {
>>> /*< private >*/
>>> SysBusDevice parent;
>>> /*< public >*/
>>> 
>>> MemoryRegion ccsr_space;
>>> +
>>> +uint32_t law_regs[NR_LAWS * 2];
>>> };
>>> 
>>> #define TYPE_CCSR "e500-ccsr"
>>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>>> index 64f8c766b4..376cb4cb37 100644
>>> --- a/hw/ppc/e500.c
>>> +++ b/hw/ppc/e500.c
>>> @@ -43,6 +43,7 @@
>>> #include "qemu/host-utils.h"
>>> #include "qemu/option.h"
>>> #include "hw/pci-host/ppce500.h"
>>> +#include "qemu/log.h"
>>> #include "qemu/error-report.h"
>>> #include "hw/platform-bus.h"
>>> #include "hw/net/fsl_etsec/etsec.h"
>>> @@ -1331,11 +1332,83 @@ void ppce500_init(MachineState *machine)
>>> boot_info->dt_size = dt_size;
>>> }
>>> 
>>> +static int law_idx(hwaddr addr)
>>> +{
>>> +int idx;
>>> +
>>> +addr -= 0xc08;
>>> +idx = 2 * ((addr >> 5) & 0xf);
>>> +if (addr & 8) {
>>> +idx++;
>>> +}
>>> +assert(idx < 2 * NR_LAWS);
>>> +return idx;
>>> +}
>>> +
>>> +static uint64_t law_read(void *opaque, hwaddr addr, unsigned size)
>>> +{
>>> +PPCE500CCSRState *s = opaque;
>>> +uint64_t val = 0;
>>> +
>>> +switch (addr) {
>>> +case 0:
>>> +val = s->ccsr_space.addr >> 12;
>>> +break;
>>> +case 0xc08 ... 0xd70:
>>> +val = s->law_regs[law_idx(addr)];
>>> +break;
>>> +default:
>>> +qemu_log_mask(LOG_GUEST_ERROR, "Invalid local access register read"
>>> +  "0x%" HWADDR_PRIx "\n", addr);
>>> +}
>>> +return val;
>>> +}
>>> +
>>> +static void law_write(void *opaque, hwaddr addr, uint64_t val, unsigned 
>>> size)
>>> +{
>>> +PPCE500CCSRState *s = opaque;
>>> +
>>> +switch (addr) {
>>> +case 0:
>>> +val &= 0x00;
>>> +memory_region_set_address(&s->ccsr_space, val << 12);
>>> +break;
>>> +case 0xc08 ... 0xd70:
>>> +{
>>> +int idx = law_idx(addr);
>>> +
>>> +qemu_log_mask(LOG_UNIMP, "Unimplemented local access register 
>>> write"
>>> +  "0x%" HWADDR_PRIx " <- 0x%" PRIx64 "\n", addr, val);
>>> +val &= (idx & 1) ? 0x80f0003f : 0xff;
>>> +s->law_regs[idx] = val;
>>> +break;
>>> +}
>>> +default:
>>> +qemu_log_mask(LOG_GUEST_ERROR, "Invalid local access register 
>>> write"
>>> +  "0x%" HWADDR_PRIx "\n", addr);
>>> +}
>>> +}
>>> +
>>> +static const MemoryRegionOps law_ops = {
>>> +.read = law_read,
>>> +.write = law_write,
>>> +.endianness = DEVICE_BIG_ENDIAN,
>>> +.valid = {
>>> +.min_access_size = 4,
>>> +.max_access_size = 4,
>>> +},
>>> +};
>>> +
>>> static void e500_ccsr_initfn(Object *obj)
>>> {
>>> -PPCE500CCSRState *ccsr = CCSR(obj);
>>> -memory_region_init(&ccsr->ccsr_space, obj, "e500-ccsr",
>>> -   MPC8544_CCSRBAR_SIZE);
>>> +PPCE500CCSRState *s = CCSR(obj);
>>> +MemoryRegion *mr;
>>> +
>>> +memory_region_init(&s->ccsr_space, obj, "e500-ccsr", 
>>> MPC8544_CCSRBAR_SIZE);
>>> +
>>> +mr = g_new(MemoryRegion, 1);
>>> +memory_region_init_io(mr, obj, &law_ops, s, "local-access", 4096);
>>> +memory_region_add_subregion(&s->ccsr_space, 0, mr);
>>> }
>>> 
>>> static const TypeInfo e500_ccsr_info = {
>>> 

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[Bernhard Beschow]

Am 30. Januar 2025 12:45:58 UTC schrieb BALATON Zoltan :
>On Wed, 15 Jan 2025, BALATON Zoltan wrote:
>> This allows guests to set the CCSR base address. Also store and return
>> values of the local access window registers but their functionality
>> isn't implemented.
>
>Ping?

I guess you're trying to boot a real firmware image from SD card? I've 
implemented that in my e500-fdt branch which I want to send as an RFC. I still 
need to clean it up. Once it's on the list we could make a plan how to turn it 
into a p10xx. Would that work for you?

Best regards,
Bernhard

P.S. The last commit teaches you how to start a firmware from SD card.

>
>> Signed-off-by: BALATON Zoltan 
>> ---
>> hw/ppc/e500-ccsr.h |  4 +++
>> hw/ppc/e500.c  | 79 --
>> 2 files changed, 80 insertions(+), 3 deletions(-)
>> 
>> diff --git a/hw/ppc/e500-ccsr.h b/hw/ppc/e500-ccsr.h
>> index 249c17be3b..cfbf96e181 100644
>> --- a/hw/ppc/e500-ccsr.h
>> +++ b/hw/ppc/e500-ccsr.h
>> @@ -4,12 +4,16 @@
>> #include "hw/sysbus.h"
>> #include "qom/object.h"
>> 
>> +#define NR_LAWS 12
>> +
>> struct PPCE500CCSRState {
>> /*< private >*/
>> SysBusDevice parent;
>> /*< public >*/
>> 
>> MemoryRegion ccsr_space;
>> +
>> +uint32_t law_regs[NR_LAWS * 2];
>> };
>> 
>> #define TYPE_CCSR "e500-ccsr"
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 64f8c766b4..376cb4cb37 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -43,6 +43,7 @@
>> #include "qemu/host-utils.h"
>> #include "qemu/option.h"
>> #include "hw/pci-host/ppce500.h"
>> +#include "qemu/log.h"
>> #include "qemu/error-report.h"
>> #include "hw/platform-bus.h"
>> #include "hw/net/fsl_etsec/etsec.h"
>> @@ -1331,11 +1332,83 @@ void ppce500_init(MachineState *machine)
>> boot_info->dt_size = dt_size;
>> }
>> 
>> +static int law_idx(hwaddr addr)
>> +{
>> +int idx;
>> +
>> +addr -= 0xc08;
>> +idx = 2 * ((addr >> 5) & 0xf);
>> +if (addr & 8) {
>> +idx++;
>> +}
>> +assert(idx < 2 * NR_LAWS);
>> +return idx;
>> +}
>> +
>> +static uint64_t law_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +PPCE500CCSRState *s = opaque;
>> +uint64_t val = 0;
>> +
>> +switch (addr) {
>> +case 0:
>> +val = s->ccsr_space.addr >> 12;
>> +break;
>> +case 0xc08 ... 0xd70:
>> +val = s->law_regs[law_idx(addr)];
>> +break;
>> +default:
>> +qemu_log_mask(LOG_GUEST_ERROR, "Invalid local access register read"
>> +  "0x%" HWADDR_PRIx "\n", addr);
>> +}
>> +return val;
>> +}
>> +
>> +static void law_write(void *opaque, hwaddr addr, uint64_t val, unsigned 
>> size)
>> +{
>> +PPCE500CCSRState *s = opaque;
>> +
>> +switch (addr) {
>> +case 0:
>> +val &= 0x00;
>> +memory_region_set_address(&s->ccsr_space, val << 12);
>> +break;
>> +case 0xc08 ... 0xd70:
>> +{
>> +int idx = law_idx(addr);
>> +
>> +qemu_log_mask(LOG_UNIMP, "Unimplemented local access register write"
>> +  "0x%" HWADDR_PRIx " <- 0x%" PRIx64 "\n", addr, val);
>> +val &= (idx & 1) ? 0x80f0003f : 0xff;
>> +s->law_regs[idx] = val;
>> +break;
>> +}
>> +default:
>> +qemu_log_mask(LOG_GUEST_ERROR, "Invalid local access register write"
>> +  "0x%" HWADDR_PRIx "\n", addr);
>> +}
>> +}
>> +
>> +static const MemoryRegionOps law_ops = {
>> +.read = law_read,
>> +.write = law_write,
>> +.endianness = DEVICE_BIG_ENDIAN,
>> +.valid = {
>> +.min_access_size = 4,
>> +.max_access_size = 4,
>> +},
>> +};
>> +
>> static void e500_ccsr_initfn(Object *obj)
>> {
>> -PPCE500CCSRState *ccsr = CCSR(obj);
>> -memory_region_init(&ccsr->ccsr_space, obj, "e500-ccsr",
>> -   MPC8544_CCSRBAR_SIZE);
>> +PPCE500CCSRState *s = CCSR(obj);
>> +MemoryRegion *mr;
>> +
>> +memory_region_init(&s->ccsr_space, obj, "e500-ccsr", 
>> MPC8544_CCSRBAR_SIZE);
>> +
>> +mr = g_new(MemoryRegion, 1);
>> +memory_region_init_io(mr, obj, &law_ops, s, "local-access", 4096);
>> +memory_region_add_subregion(&s->ccsr_space, 0, mr);
>> }
>> 
>> static const TypeInfo e500_ccsr_info = {
>> 

Re: [PATCH] hw/ppc/e500: Partial implementation of local access window registers

[BALATON Zoltan]

On Wed, 15 Jan 2025, BALATON Zoltan wrote:

This allows guests to set the CCSR base address. Also store and return
values of the local access window registers but their functionality
isn't implemented.


Ping?


Signed-off-by: BALATON Zoltan 
---
hw/ppc/e500-ccsr.h |  4 +++
hw/ppc/e500.c  | 79 --
2 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/e500-ccsr.h b/hw/ppc/e500-ccsr.h
index 249c17be3b..cfbf96e181 100644
--- a/hw/ppc/e500-ccsr.h
+++ b/hw/ppc/e500-ccsr.h
@@ -4,12 +4,16 @@
#include "hw/sysbus.h"
#include "qom/object.h"

+#define NR_LAWS 12
+
struct PPCE500CCSRState {
/*< private >*/
SysBusDevice parent;
/*< public >*/

MemoryRegion ccsr_space;
+
+uint32_t law_regs[NR_LAWS * 2];
};

#define TYPE_CCSR "e500-ccsr"
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 64f8c766b4..376cb4cb37 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -43,6 +43,7 @@
#include "qemu/host-utils.h"
#include "qemu/option.h"
#include "hw/pci-host/ppce500.h"
+#include "qemu/log.h"
#include "qemu/error-report.h"
#include "hw/platform-bus.h"
#include "hw/net/fsl_etsec/etsec.h"
@@ -1331,11 +1332,83 @@ void ppce500_init(MachineState *machine)
boot_info->dt_size = dt_size;
}

+static int law_idx(hwaddr addr)
+{
+int idx;
+
+addr -= 0xc08;
+idx = 2 * ((addr >> 5) & 0xf);
+if (addr & 8) {
+idx++;
+}
+assert(idx < 2 * NR_LAWS);
+return idx;
+}
+
+static uint64_t law_read(void *opaque, hwaddr addr, unsigned size)
+{
+PPCE500CCSRState *s = opaque;
+uint64_t val = 0;
+
+switch (addr) {
+case 0:
+val = s->ccsr_space.addr >> 12;
+break;
+case 0xc08 ... 0xd70:
+val = s->law_regs[law_idx(addr)];
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "Invalid local access register read"
+  "0x%" HWADDR_PRIx "\n", addr);
+}
+return val;
+}
+
+static void law_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+PPCE500CCSRState *s = opaque;
+
+switch (addr) {
+case 0:
+val &= 0x00;
+memory_region_set_address(&s->ccsr_space, val << 12);
+break;
+case 0xc08 ... 0xd70:
+{
+int idx = law_idx(addr);
+
+qemu_log_mask(LOG_UNIMP, "Unimplemented local access register write"
+  "0x%" HWADDR_PRIx " <- 0x%" PRIx64 "\n", addr, val);
+val &= (idx & 1) ? 0x80f0003f : 0xff;
+s->law_regs[idx] = val;
+break;
+}
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "Invalid local access register write"
+  "0x%" HWADDR_PRIx "\n", addr);
+}
+}
+
+static const MemoryRegionOps law_ops = {
+.read = law_read,
+.write = law_write,
+.endianness = DEVICE_BIG_ENDIAN,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+};
+
static void e500_ccsr_initfn(Object *obj)
{
-PPCE500CCSRState *ccsr = CCSR(obj);
-memory_region_init(&ccsr->ccsr_space, obj, "e500-ccsr",
-   MPC8544_CCSRBAR_SIZE);
+PPCE500CCSRState *s = CCSR(obj);
+MemoryRegion *mr;
+
+memory_region_init(&s->ccsr_space, obj, "e500-ccsr", MPC8544_CCSRBAR_SIZE);
+
+mr = g_new(MemoryRegion, 1);
+memory_region_init_io(mr, obj, &law_ops, s, "local-access", 4096);
+memory_region_add_subregion(&s->ccsr_space, 0, mr);
}

static const TypeInfo e500_ccsr_info = {