On 20/04/2023 18:18, BALATON Zoltan wrote:
On Thu, 20 Apr 2023, Mark Cave-Ayland wrote:
On 20/04/2023 13:54, BALATON Zoltan wrote:
(cut)
I do appreciate you're only replying to the ponts below and willing to continue constructive discussion.
If you feel that the points above were not in keeping with a technical discussion, then perhaps an apology is in order?
In case you decide to change your mind or somebody else can step in as a maintainer to more sensibly manage this project, my ideas for improvements that I'd consider to elaborate on would be:
- Finish this pci method simplification by moving some more methods to Forth
I've already answered this one.
If I got your answer correctly you said this is not a good idea because you prefer to keep everyting in pci.c even if that's cluttering that file and doing some things from Forth would be simpler, because you think Forth is harder to maintain than C so OpenBIOS should have less Forth not more. Does that summarise your answer or did I misunderstand it?
No, I'm afraid you've misunderstood my reply completely.
- Implement missing register access words to get FCode ROMs working
Segher already replied to this, explaining how the implementation should call the associated FCode tokens. I don't believe there has been another version of the patch submitted that addresses his comments?
The first version I've submitted of this was done during testing passing through a card to MacOS with QEMU and later I've realised it wasn't right and also replied with that on my own patch. I then haven't done a new version because the person who was testing this disappeared and I was busy doing other things so haven't yet got around trying to impelement it again. I may want to do it in the future but the difficulty of making any change does not encourage me to do so.
Obviously we all contribute our time/effort as we see fit in a community project, so I leave it to you if you wish to submit an updated version of the patch based upon Segher's reply.
- Get rid of fw_cfg and hard coded machine parameters and pass an FDT from QEMU
instead as done my spapr and SLOF or pegasos2 and VOF; I've already ported SLOF's FDT parser to OpenBIOS but because it's impossible to convince you I did not finish this. This would simplify arch/ppc/qemu a lot by removing hard coded device trees and lift the need to change OpenBIOS together with QEMU as then device tree changes could be made in QEMU alone. For example fixing G5 mac99 can be possible without any change in OpenBIOS and thus lessen maintenance need for OpenBIOS.
When I asked how this would work with the device trees already included in OpenBIOS, in particular i) how you would bind Forth words into the device nodes in the FDT and ii) how this would work for non-QEMU builds without duplication there was no follow-up proposal.
As I said these are ideas that need to be elaborated so I may not be able to answer every detail before I get there, but if the ideas are shot down even before attempting it, I'll never get there. I think at least SLOF proves it's possible to do that and this series already explored at least two ways to bind methods to device nodes so the above should not be unsolvable problems, even if we don't yet have a clear understanding of every detail. So citing it as reason to not even consider the idea is not progressive.
The thing is we already know that passing an FDT image is possible, the big unknown is how such a binding with Forth words would work. This is a such a critical part of the solution I can't see how you would invest any significant time into this without proposing a solution.
The other thing to remember is that OpenBIOS is intended to run on real hardware where possible: I don't particularly want 2 separate mechanisms for building device trees in OpenBIOS since that then takes twice as long to debug. Will you remember to test both scenarios before you submit patches? Will I?
As you can see from this the bar to make such a feature worthwhile is fairly high, so then the next question is does such a feature save us time and effort? How often do we find that we need to change DT properties in OpenBIOS to match QEMU? I'd say it is very rare these days.
I could see that there could be potential to bring libfdt into OpenBIOS since then we can source the FDT image locally if there is no hypervisor, but again you end up back at the same critical place: how can the Forth word binding work, and in a way that isn't detrimental to developers working in Forth and C? A full solution isn't required, but you'll need a demonstrable proof-of-concept to show developers how this will work.
The point was that if the first reaction to every patch I submit will be that you don't agree and try to find how to prove it's wrong and yout idea is better, then it's really hard to contribute anything. This has certainly been my experience so far, not only in OpenBIOS but also in QEMU. The exceptions are cases where the proposed patch matched exacrly your ideas but even then you like to change it a bit and not just accept it as it is.
I don't understand this at all? Generally most reviewers will request minor changes to patches, as both you and I have done in the past.
As for the above problem, this would be solved when we get there. What I have now is that I can pass an FDT to OpenBIOS which is parsed to create an unflattened version. It's been a while I've tried it so maybe for OpenBIOS some more changes would be needed but it worked at least with Open Firmware, you can look it up on that mailing list where I've documented that experiment, Maybe the problems you mention don't even exist as this would jist replace the included device trees with one created by QEMU that contain the root node, cpus, memory and top level of pci nodes but no PCI devices (this is what's now hard coded in atch/ppc/qemu/init.c) then the rest is done the same way as now. So what I propose is simply instead of keeping a list of hardcoded stuff in OpenBIOS, read those from an FDT which is passed from QEMU. This way we don't have duplicated info about machines in both QEMU and OpenBIOS so 1) you don't have to maintain this in OpenBIOS and 2) no need to chahge both QEMU and OpenBIOS as the info is then only in the machine model in QEMU.
(already covered above, but again consider how often do we need to do this?)
(Related to this is passing VGA driver through ROM to lessen dependency on fw_cfg as a first step and move towards real FCode ROM as it works on real hardware so instead of hacks do what real hardware does. But that's another change that met resistence from you.)
As already explained on the QEMU list, the problem here is that the NDRV binary can only be built within a classic MacOS guest which means you would end up with the problems of distributing a binary blob from an external project. Since QEMU already has a mechanism to support this, it makes sense to leverage that via fw_cfg injection into OpenBIOS to avoid a dependency problem, which was also the intention of the original author (Ben).
This again isn't a problem that couldn't be solved if you wanted. What's more, it isn't even a problem yet before we can run FCodes. At this stage all I proposed was to pass the existing NDRV from QEMU to OpenBIOS putting it in the card ROM instead of downloading it from fw_cfg. This would allow simplifying OpenBIOS as then you can get rid of code to download the file and keep the NDRV dependency in QEMU.
But then you still have to build a ROM file with an embedded binary blob from an external source. I should add that the current mechanism doesn't prevent passing in external PCI option ROMs from real cards if needed, and it wouldn't have been accepted if it did.
Later if you don't want to introduce dependency on the NDRV in OpenBIOS to build the FCode ROM then do the other way around and let the NDRV depend on fcode-utils to compile the binary and the forth part to create an FCode ROM (after all that's a separate project anyway to provide a ROM for QEMU's graphics card). Then the FCode ROM will be included in QEMU the same way the ndrv is now included.
If you'd tried this, you'd immediately discover the problem trying to run fcode-utils in the non-POSIX NDRV build environment. And as previously discussed the longer-term aim is to generate a proper PCI option ROM from the FCode VGA driver, but there isn't a tool in either fcode-utils or OpenBIOS to generate the final ROM image (yet).
I don't see any insurmountable problem here, you just like to present these as one to block progress with these and keep to your ideas.
Indeed they are not insurmountable, but I've responded to them above for you, in case you would like to work on and/or discuss this further.
I could try to explain the ideas in more detail but if your first reaction is just that you don't think this can be a good solution then even spending time with detailing the ideas is wasted time so I'll just stop now.
I've replied to your points above for completeness, since your representation above is misleading and implies that I am neglecting my role as maintainer.
Let me calrify again, I did not say you're neglecting your role. What I said is that you're doing it in a way which does not let anybody else contribute to the project (or make it unnecessarily hard) which holds back progress and discourages contributions. I think this should change. The best would be if you could change but if you can't then maybe somebody else could do a better job. But I've tried to explain that several times, this is the last attempt to do that again before I give up.
There is nothing wrong with having ideas as to how the project should proceed, but as you see above these ideas were often missing follow-up patches or critical detail. And one of the roles of a maintainer is to point out such problems before anyone invests a significant amount of time or effort to avoid disappointment.
On the other hand if you shot down ideas at the beginning and don't even accept simple steps towards a bigger goal deciding in advance that it won't work without even trying then people are not encouraged to pursue ideas as it will just be wasted time if they can't improve the project in the end.
How is pointing out the problems with a proposal "shooting it down"? If you can't demonstrate how to resolve the indicated problems then it strongly suggests the proposal isn't going to work.
Finally your comments about holding the project hostage are completely unacceptable:
I know my words may sound harsh but I could not put it better what the experience I got trying to contribute to projects you're maintaining was most of the time. You listen to what I say then just completely ignore it and do the same thing again and again. I can't even avoid it because we seem to be interested in the same parts of QEMU so we'll inevitably cross roads eventually. I can quit trying to change OpenBIOS and Mac emulation in QEMU but I won't completely leave QEMU development so this may happen again and again unfortunately. I think my ideas are reasonable and would improve the project but I can't even try it because you don't believe in it so don't even let it happen.
I believe I've replied (yet again) to the various points raised in your message.
if a maintainer points out problems with your ideas or patches, it is not because of a whim but because there is a good reason e.g. the PCI bus id patches were refused because they broke booting NetBSD. If you find that you are constantly fighting with maintainers when proposing ideas and/or patches, I can only suggest taking some time out for a period of introspection to try and understand why this is the case.
I would do that if that happened and I can take reasonable criticism and improve the patches if needed but I find I only constantly have to fight with you, not with other maintainers so I believe it's your standards or views are unreasonably high not my approach is completely wrong. If the problems pointed out are real problems then the review helps. If they are only stem from constant disagreement and different views or look to me like excuses to not consider the changes then I have a hard time accepting that.
From what I can see all my arguments above are technical: if not, what have I missed?
(I've actually debugged that NetBSD issue back then and found it only boots now because it happens to work by chance as some memory that it used wasn't overwritten before but the slightly changed memory layout after the patch ended up overlapping something the guest used so that patch did not actually broke anything that wasn't already broken just exposed and existing ptoblem so there wasn't much to fix in that patch and I wasn't willing to debug and fix unrelated problems, especially that the latest NetBSD version still booted, it was only an old, likely unused version that had a problem.)
If you don't want to fix the issue, then that's fine. However I'm not willing to merge a patch that breaks things, particularly as we have no idea how many installations of NetBSD will be affected. Even more so, since I'm the maintainer then it will be me that gets the complaints both on and off-list.
ATB,
Mark.