[PATCH v4 00/19] Misc ppc/mac machines clean up

BALATON Zoltan posted 19 patches 1 year, 6 months ago
Failed in applying to current master (apply log)
Maintainers: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, John Snow <jsnow@redhat.com>
There is a newer version of this series
MAINTAINERS                   |   2 +
docs/about/deprecated.rst     |   7 +
docs/system/ppc/powermac.rst  |  12 +-
hw/ide/macio.c                |   1 -
hw/intc/heathrow_pic.c        |   1 -
hw/intc/openpic.c             |   1 -
hw/misc/macio/cuda.c          |   1 -
hw/misc/macio/gpio.c          |   1 -
hw/misc/macio/macio.c         |   8 +-
hw/misc/macio/pmu.c           |   1 -
hw/nvram/mac_nvram.c          |   2 +-
hw/pci-host/grackle.c         |  15 +-
hw/pci-host/uninorth.c        |   1 -
hw/ppc/mac.h                  | 105 -----------
hw/ppc/mac_newworld.c         | 341 ++++++++++++++++++++++------------
hw/ppc/mac_oldworld.c         | 120 ++++++------
include/hw/misc/macio/macio.h |  23 ++-
include/hw/nvram/mac_nvram.h  |  51 +++++
include/hw/pci-host/grackle.h |  44 +++++
19 files changed, 415 insertions(+), 322 deletions(-)
delete mode 100644 hw/ppc/mac.h
create mode 100644 include/hw/nvram/mac_nvram.h
create mode 100644 include/hw/pci-host/grackle.h
[PATCH v4 00/19] Misc ppc/mac machines clean up
Posted by BALATON Zoltan 1 year, 6 months ago
Since only one week is left until freeze starts I've included some
more patches in this version that I've intended to submit after the
clean ups but we're running out of time now. The last 3 patches could
be squashed together, I've just split these up because I expect
resistence from Mark to any changes so maybe it's easier to digest
piece by piece and can cherry pick parts easier this way but ideally
these should be in one patch.

I'd appreciate very much if this series would get in before the
freeze, it is very discouraging to spend time with something that gets
ignored and then postponed for the rest of the year just to start
again the same in January. This might be a reason why not many people
contribute to this part of QEMU besides that maybe only a few people
are still interested so those who are interested should be served
better to not scare them off even more.

Regards,
BALATON Zoltan

v4: Add some more patches that I've found since v3 or was intended in
separate series
v3: Some more patch spliting and changes I've noticed and address more
review comments
v2: Split some patches and add a few more I've noticed now and address
review comments

BALATON Zoltan (19):
  mac_newworld: Drop some variables
  mac_oldworld: Drop some more variables
  mac_{old|new}world: Set tbfreq at declaration
  mac_{old|new}world: Avoid else branch by setting default value
  mac_{old|new}world: Simplify cmdline_base calculation
  mac_newworld: Clean up creation of Uninorth devices
  mac_{old|new}world: Reduce number of QOM casts
  hw/ppc/mac.h: Move newworld specific parts out from shared header
  hw/ppc/mac.h: Move macio specific parts out from shared header
  hw/ppc/mac.h: Move grackle-pcihost type declaration out to a header
  hw/ppc/mac.h: Move PROM and KERNEL defines to board code
  hw/ppc/mac.h: Rename to include/hw/nvram/mac_nvram.h
  mac_nvram: Use NVRAM_SIZE constant
  mac_{old|new}world: Code style fix adding missing braces to if-s
  mac_newworld: Turn CORE99_VIA_CONFIG defines into an enum
  mac_newworld: Add machine types for different mac99 configs
  mac_newworld: Deprecate mac99 with G5 CPU
  mac_newworld: Deprecate mac99 "via" option
  mac_newworld: Document deprecation

 MAINTAINERS                   |   2 +
 docs/about/deprecated.rst     |   7 +
 docs/system/ppc/powermac.rst  |  12 +-
 hw/ide/macio.c                |   1 -
 hw/intc/heathrow_pic.c        |   1 -
 hw/intc/openpic.c             |   1 -
 hw/misc/macio/cuda.c          |   1 -
 hw/misc/macio/gpio.c          |   1 -
 hw/misc/macio/macio.c         |   8 +-
 hw/misc/macio/pmu.c           |   1 -
 hw/nvram/mac_nvram.c          |   2 +-
 hw/pci-host/grackle.c         |  15 +-
 hw/pci-host/uninorth.c        |   1 -
 hw/ppc/mac.h                  | 105 -----------
 hw/ppc/mac_newworld.c         | 341 ++++++++++++++++++++++------------
 hw/ppc/mac_oldworld.c         | 120 ++++++------
 include/hw/misc/macio/macio.h |  23 ++-
 include/hw/nvram/mac_nvram.h  |  51 +++++
 include/hw/pci-host/grackle.h |  44 +++++
 19 files changed, 415 insertions(+), 322 deletions(-)
 delete mode 100644 hw/ppc/mac.h
 create mode 100644 include/hw/nvram/mac_nvram.h
 create mode 100644 include/hw/pci-host/grackle.h

-- 
2.30.4
Re: [PATCH v4 00/19] Misc ppc/mac machines clean up
Posted by Howard Spoelstra 1 year, 6 months ago
On Tue, Oct 25, 2022 at 6:49 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:

> Since only one week is left until freeze starts I've included some
> more patches in this version that I've intended to submit after the
> clean ups but we're running out of time now. The last 3 patches could
> be squashed together, I've just split these up because I expect
> resistence from Mark to any changes so maybe it's easier to digest
> piece by piece and can cherry pick parts easier this way but ideally
> these should be in one patch.
>
> I'd appreciate very much if this series would get in before the
> freeze, it is very discouraging to spend time with something that gets
> ignored and then postponed for the rest of the year just to start
> again the same in January. This might be a reason why not many people
> contribute to this part of QEMU besides that maybe only a few people
> are still interested so those who are interested should be served
> better to not scare them off even more.
>
> Regards,
> BALATON Zoltan
>
> v4: Add some more patches that I've found since v3 or was intended in
> separate series
> v3: Some more patch spliting and changes I've noticed and address more
> review comments
> v2: Split some patches and add a few more I've noticed now and address
> review comments
>
> BALATON Zoltan (19):
>   mac_newworld: Drop some variables
>   mac_oldworld: Drop some more variables
>   mac_{old|new}world: Set tbfreq at declaration
>   mac_{old|new}world: Avoid else branch by setting default value
>   mac_{old|new}world: Simplify cmdline_base calculation
>   mac_newworld: Clean up creation of Uninorth devices
>   mac_{old|new}world: Reduce number of QOM casts
>   hw/ppc/mac.h: Move newworld specific parts out from shared header
>   hw/ppc/mac.h: Move macio specific parts out from shared header
>   hw/ppc/mac.h: Move grackle-pcihost type declaration out to a header
>   hw/ppc/mac.h: Move PROM and KERNEL defines to board code
>   hw/ppc/mac.h: Rename to include/hw/nvram/mac_nvram.h
>   mac_nvram: Use NVRAM_SIZE constant
>   mac_{old|new}world: Code style fix adding missing braces to if-s
>   mac_newworld: Turn CORE99_VIA_CONFIG defines into an enum
>   mac_newworld: Add machine types for different mac99 configs
>   mac_newworld: Deprecate mac99 with G5 CPU
>   mac_newworld: Deprecate mac99 "via" option
>   mac_newworld: Document deprecation
>
>  MAINTAINERS                   |   2 +
>  docs/about/deprecated.rst     |   7 +
>  docs/system/ppc/powermac.rst  |  12 +-
>  hw/ide/macio.c                |   1 -
>  hw/intc/heathrow_pic.c        |   1 -
>  hw/intc/openpic.c             |   1 -
>  hw/misc/macio/cuda.c          |   1 -
>  hw/misc/macio/gpio.c          |   1 -
>  hw/misc/macio/macio.c         |   8 +-
>  hw/misc/macio/pmu.c           |   1 -
>  hw/nvram/mac_nvram.c          |   2 +-
>  hw/pci-host/grackle.c         |  15 +-
>  hw/pci-host/uninorth.c        |   1 -
>  hw/ppc/mac.h                  | 105 -----------
>  hw/ppc/mac_newworld.c         | 341 ++++++++++++++++++++++------------
>  hw/ppc/mac_oldworld.c         | 120 ++++++------
>  include/hw/misc/macio/macio.h |  23 ++-
>  include/hw/nvram/mac_nvram.h  |  51 +++++
>  include/hw/pci-host/grackle.h |  44 +++++
>  19 files changed, 415 insertions(+), 322 deletions(-)
>  delete mode 100644 hw/ppc/mac.h
>  create mode 100644 include/hw/nvram/mac_nvram.h
>  create mode 100644 include/hw/pci-host/grackle.h
>
> --
> 2.30.4
>
>
>
Hi all,

I applied these patches and they seem to work as expected. I like the way
this makes it clearer which machine is actually emulated, even though it is
still not easy to understand which default hardware the emulated machine
actually presents.
I also like the more consistent way a new rom file for a VGA device can be
added. The deprecation warnings are clear.

Qemu-system-ppc defaults to the g3beige machine, which does not reflect the
(in my opinion) main use case of running Mac OS/X with the powermac3_1
machine and will not boot the main versions of ppc Mac OS/X anyway.

So for qemu-system-ppc:

Tested-by: Howard Spoelstra <hsp.cat7@gmail.com>

Best,
Howard
Re: [PATCH v4 00/19] Misc ppc/mac machines clean up
Posted by BALATON Zoltan 1 year, 6 months ago
On Thu, 27 Oct 2022, Howard Spoelstra wrote:
> I applied these patches and they seem to work as expected. I like the way
> this makes it clearer which machine is actually emulated, even though it is
> still not easy to understand which default hardware the emulated machine
> actually presents.

Thanks for the feed back and testing. The emulation is not perfect so 
there are some differences from the actual machines. These could be 
documented in qemu/docs/system/ppc/powermac.rst patches are welcome). Some 
of these are not yet implemented like sound or i2c (see: 
https://osdn.net/projects/qmiga/wiki/SubprojectMac99I2C and 
https://patchew.org/QEMU/cover.1593456926.git.balaton@eik.bme.hu/93758f65ef21d977fe835364bb1386fb4c03a6ce.1593456926.git.balaton@eik.bme.hu/ 
if anybody is interested to finish these) or some are missing due to 
OpenBIOS can't yet handle it like a PCI bridge on some PCI bus which was 
there in code commented out for a while but looks like it's gone now or I 
couldn't find it. But the presented hardware should be close enough to 
these machines for OSes and it also shows what machines we should aim for 
so it's not an undefined machine any more. The mac99 machine may not be an 
actual existing config, according to

http://macos9lives.com/smforum/index.php/topic,2408.msg28843.html?PHPSESSID=ce15448df7a74e13c82c59eedf624db7#msg28843

which says no Mac had Uninorth, Keylargo and CUDA, although this forum 
post may not list every machine, e.g. powermac1,2 (the first PCI Power Mac 
G4) according to <https://en.wikipedia.org/wiki/Power_Mac_G4> had CUDA but 
used Grackle (the same motherboard as the Blue&White G3 PowerMac 
powermac1,1 <https://en.wikipedia.org/wiki/Power_Macintosh_G3_(Blue_and_White)> )
but had no ADB ports so you could not have ADB keyboard and mouse attached 
to it like we have in mac99. The powermac1,2 is maybe more similar to 
g3beige but g3beige has old world ROM while the B&W G3 powermac1,1 is the 
first new world ROM machine but may have more differences I don't know 
about. (That also means maybe our naming mac_oldworld and mac_newworld is 
misleading but that's OK for now as it's only in the source code and not 
user visible.)

> I also like the more consistent way a new rom file for a VGA device can be
> added. The deprecation warnings are clear.

Some more info on this last ndrv via romfile patch: OpenBIOS has two ways 
to add an NDRV in the device tree for MacOS to a vga card:

1. It adds it in openbios/drivers/pci.c::vga_config_cb() if the ROM 
contains an NDRV

2. Then in vga-driver-fcode defined in vga.fs (that OpenBIOS 
unconditionally calls for vga devices it knows about) it also checks for a 
file called ndrv/qemu_vga.ndrv in fw_cfg and adds that to the device tree. 
The vga-ndrv? option controls this second way and defaults to true.

Problems with 2.

- The ndrv/qemu_vga.ndrv is added by the machine not the card so it will 
be used for other cards (liek ati-vga) that it shouldn't be used for and 
there's no good way to control or fix it other than the user having to set 
vga-ndrv? to false when adding -device ati-vga.

- It's too complex for no good reason so after my patch this could be 
dropped altogether simpifying the code both in QEMU and OpenBIOS.

My patch sets the default value for the romfile property of the VGA device 
to qemu_vga.ndrv instead so QEMU will put the ndrv in the ROM and OpenBIOS 
detects that and adds it to the property without going through fw_cfg (it 
still checks fw_cfg but since we don't add the ndrv there any more that 
part won't do anything so that can be dropped later from OpenBIOS together 
with the vga-ndrv? option. If you want to disable the ndrv with my patch 
you can use -device VGA,romfile="" instead which replaces the default with 
empty romfile so OpenBIOS won't find it neither in the ROM not in fw_cfg. 
Additionally you can pass a real FCode ROM or different NDRV the same way 
via romfile now without having to replace the file in QEMU install which 
might come handy for someone developing NDRVs or experimenting with ROMs 
or pass-thorugh. So I think this simple patch really helps users and makes 
the code overall simpler too.

> Qemu-system-ppc defaults to the g3beige machine, which does not reflect the
> (in my opinion) main use case of running Mac OS/X with the powermac3_1
> machine and will not boot the main versions of ppc Mac OS/X anyway.

We can't easily change the default wihtout breaking existing commands and 
it's also debatable what should be a new default so I think we're stuck 
with that now. In any case we need an at least 2 release long deprecation 
period so what we could do is to deprecare g3beige as the default to 
require users to always specify a machine option explicitly so we can do 
something with it in the future but I don't know how to add such warning, 
i.e. how to detect if g3beige was chosen via -M or by default. Maybe this 
warning should be issued by command line parsing not the g3beige board 
code? So I've only added warnings for the mac99 with via option and G5 CPU 
for now and left qemu-system-ppc -M mac99 and g3beige alone for now. If 
you think these also need some warnings added now then we should find out 
how and what should be done instead. I could not decide on those so opted 
for preserving backwards compatibility for these.

Regards,
BALATON Zoltan

> So for qemu-system-ppc:
>
> Tested-by: Howard Spoelstra <hsp.cat7@gmail.com>
>
> Best,
> Howard
Re: [PATCH v4 00/19] Misc ppc/mac machines clean up
Posted by BALATON Zoltan 1 year, 6 months ago
On Tue, 25 Oct 2022, BALATON Zoltan wrote:
> Since only one week is left until freeze starts I've included some
> more patches in this version that I've intended to submit after the
> clean ups but we're running out of time now. The last 3 patches could
> be squashed together, I've just split these up because I expect
> resistence from Mark to any changes so maybe it's easier to digest
> piece by piece and can cherry pick parts easier this way but ideally
> these should be in one patch.
>
> I'd appreciate very much if this series would get in before the
> freeze, it is very discouraging to spend time with something that gets
> ignored and then postponed for the rest of the year just to start
> again the same in January. This might be a reason why not many people
> contribute to this part of QEMU besides that maybe only a few people
> are still interested so those who are interested should be served
> better to not scare them off even more.

Found a typo in the last (docs) patch so resent a v5 for just this patch 
and added another patch that could go in now as it works without OpenBIOS 
changes now so changes this allows in OpenBIOS can be done later at any 
time independently but it fixes the problem with ati-vga and qemu_vga.ndrv 
so it's useful for that alone.

Regards,
BALATON Zoltan

> v4: Add some more patches that I've found since v3 or was intended in
> separate series
> v3: Some more patch spliting and changes I've noticed and address more
> review comments
> v2: Split some patches and add a few more I've noticed now and address
> review comments
>
> BALATON Zoltan (19):
>  mac_newworld: Drop some variables
>  mac_oldworld: Drop some more variables
>  mac_{old|new}world: Set tbfreq at declaration
>  mac_{old|new}world: Avoid else branch by setting default value
>  mac_{old|new}world: Simplify cmdline_base calculation
>  mac_newworld: Clean up creation of Uninorth devices
>  mac_{old|new}world: Reduce number of QOM casts
>  hw/ppc/mac.h: Move newworld specific parts out from shared header
>  hw/ppc/mac.h: Move macio specific parts out from shared header
>  hw/ppc/mac.h: Move grackle-pcihost type declaration out to a header
>  hw/ppc/mac.h: Move PROM and KERNEL defines to board code
>  hw/ppc/mac.h: Rename to include/hw/nvram/mac_nvram.h
>  mac_nvram: Use NVRAM_SIZE constant
>  mac_{old|new}world: Code style fix adding missing braces to if-s
>  mac_newworld: Turn CORE99_VIA_CONFIG defines into an enum
>  mac_newworld: Add machine types for different mac99 configs
>  mac_newworld: Deprecate mac99 with G5 CPU
>  mac_newworld: Deprecate mac99 "via" option
>  mac_newworld: Document deprecation
>
> MAINTAINERS                   |   2 +
> docs/about/deprecated.rst     |   7 +
> docs/system/ppc/powermac.rst  |  12 +-
> hw/ide/macio.c                |   1 -
> hw/intc/heathrow_pic.c        |   1 -
> hw/intc/openpic.c             |   1 -
> hw/misc/macio/cuda.c          |   1 -
> hw/misc/macio/gpio.c          |   1 -
> hw/misc/macio/macio.c         |   8 +-
> hw/misc/macio/pmu.c           |   1 -
> hw/nvram/mac_nvram.c          |   2 +-
> hw/pci-host/grackle.c         |  15 +-
> hw/pci-host/uninorth.c        |   1 -
> hw/ppc/mac.h                  | 105 -----------
> hw/ppc/mac_newworld.c         | 341 ++++++++++++++++++++++------------
> hw/ppc/mac_oldworld.c         | 120 ++++++------
> include/hw/misc/macio/macio.h |  23 ++-
> include/hw/nvram/mac_nvram.h  |  51 +++++
> include/hw/pci-host/grackle.h |  44 +++++
> 19 files changed, 415 insertions(+), 322 deletions(-)
> delete mode 100644 hw/ppc/mac.h
> create mode 100644 include/hw/nvram/mac_nvram.h
> create mode 100644 include/hw/pci-host/grackle.h
>
>
Re: [PATCH v4 00/19] Misc ppc/mac machines clean up
Posted by Mark Cave-Ayland 1 year, 6 months ago
On 25/10/2022 17:44, BALATON Zoltan wrote:

> Since only one week is left until freeze starts I've included some
> more patches in this version that I've intended to submit after the
> clean ups but we're running out of time now. The last 3 patches could
> be squashed together, I've just split these up because I expect
> resistence from Mark to any changes so maybe it's easier to digest
> piece by piece and can cherry pick parts easier this way but ideally
> these should be in one patch.
> 
> I'd appreciate very much if this series would get in before the
> freeze, it is very discouraging to spend time with something that gets
> ignored and then postponed for the rest of the year just to start
> again the same in January. This might be a reason why not many people
> contribute to this part of QEMU besides that maybe only a few people
> are still interested so those who are interested should be served
> better to not scare them off even more.
> 
> Regards,
> BALATON Zoltan
> 
> v4: Add some more patches that I've found since v3 or was intended in
> separate series
> v3: Some more patch spliting and changes I've noticed and address more
> review comments
> v2: Split some patches and add a few more I've noticed now and address
> review comments

Oh wait, there's already a v4 with even more changes in? This is really confusing as 
a reviewer...


ATB,

Mark.
Re: [PATCH v4 00/19] Misc ppc/mac machines clean up
Posted by BALATON Zoltan 1 year, 6 months ago
On Fri, 28 Oct 2022, Mark Cave-Ayland wrote:
> On 25/10/2022 17:44, BALATON Zoltan wrote:
>> Since only one week is left until freeze starts I've included some
>> more patches in this version that I've intended to submit after the
>> clean ups but we're running out of time now. The last 3 patches could
>> be squashed together, I've just split these up because I expect
>> resistence from Mark to any changes so maybe it's easier to digest
>> piece by piece and can cherry pick parts easier this way but ideally
>> these should be in one patch.
>> 
>> I'd appreciate very much if this series would get in before the
>> freeze, it is very discouraging to spend time with something that gets
>> ignored and then postponed for the rest of the year just to start
>> again the same in January. This might be a reason why not many people
>> contribute to this part of QEMU besides that maybe only a few people
>> are still interested so those who are interested should be served
>> better to not scare them off even more.
>> 
>> Regards,
>> BALATON Zoltan
>> 
>> v4: Add some more patches that I've found since v3 or was intended in
>> separate series
>> v3: Some more patch spliting and changes I've noticed and address more
>> review comments
>> v2: Split some patches and add a few more I've noticed now and address
>> review comments
>
> Oh wait, there's already a v4 with even more changes in? This is really 
> confusing as a reviewer...

I've intended to submit these last patches as a separate series after the 
simple clean ups but that series were under review for so long that I had 
to include these now to have a chance to meet the freeze deadline. Ideally 
the simple clean ups should not have taken more than 2 weeks then we had 
another 2 weeks for these.

Regards,
BALATON Zoltan