Re: [Qemu-arm] [Qemu-devel] [PATCH] BCM2837 and machine raspi3

[Andrew Baumann]

> From: bzt bzt [mailto:address@hidden
> Sent: Tuesday, 24 October 2017 02:54
> On Mon, Oct 23, 2017 at 6:34 PM, Andrew Baumann wrote:
>       Are there any changes from bcm2836 other than the CPU model?
> 
> 
>       Duplicating the whole file just to have a different CPU seems like a bad
> idea to me. I would suggest parameterising the CPU model in bcm2836 (and
> maybe noting in header comments etc. that it can also model 2837), and then
> instantiating it from raspi.c with the correct CPU type depending on which
> machine is being used. That will also reduce the size of the patch 
> significantly,
> which will make it easier to get it reviewed.
> 
> 
> 
> For now, there's no more difference in the emulation than the CPU, but
> BCM2837 has a lot more to offer (40 bit address space, wifi, bluetooth etc.). 
> So
> sooner or later it will need it's own source file to support all of that 
> without
> getting the code messy. I have asked Peter whether it does worth writing
> future proof code, or keep the change at the bare minimum, but had no answer
> yet.

I see. The address space size sounds like it would affect the SoC (although is 
there really 40 bits of usable physical address space beyond the core?). If 
it's like pi2, however, the wifi and BT are both behind USB, so that would be 
handled more naturally in the board model (raspi.c) than the SoC.

> Well, bcm2837 is backward compatible with the bcm2836, but has a lot more
> registers and a different boot up process (different address and no atags). 
> The
> common functionality is already in bcm2835_peripherals, and the MMIO
> address change from bcm2835 to bcm2836 has already been added by you.
> What's different is a lot more devices, which imho would be unwise to add to
> bcm2835 or either to bcm2836. So to be future proof I've created a separated
> file, but you are right at the current level of emulation it's not necessary.

Right, but as I wrote above if those devices are behind USB that's not part of 
the SoC model, and belongs in the board logic. If "more registers" just refers 
to the CPU, then that's irrelevant. If there are really more devices / device 
registers on the system bus, then that calls for a deeper change in the SoC 
model and perhaps a new implementation.

>       Similarly, this looks like a clone of raspi2_init. I think it would be 
> better
> to have one function, parametrised if needed.
> 
> 
> 
> For now, yes. But the extra devices bcm2837 have will need extra 
> initialization,
> and I though it would be clearer to separate in advance. Again, I've asked 
> Peter
> about this, but had no response. As I see having a parametrised single 
> function
> is simplier for now, but could lead to a messy code later when all the SoC
> features will be added. But I'm ain't no expert on qemu source, this is my 
> first
> patch, so I'm open to suggestions! :-)

I'm more open to the need for evolution in the machine init logic (raspi.c), so 
splitting there makes more sense to me than in bcm2836/7.

I see you just posted another patch. FWIW, this isn't quite what I was 
proposing -- rather than have bcm2736.c take a version number that is 2 or 3, I 
would just pass it the CPU model to instantiate. But at this point I think it's 
best waiting for one of the Qemu maintainers to chime in and see what they 
prefer.

Andrew