On 27/04/2023 15:21, BALATON Zoltan wrote:
On Wed, 26 Apr 2023, Mark Cave-Ayland wrote:
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?
If you feel an apology is needed then I'm happy to say sorry again but what I meant was that the above were just describing the current state and past so discussing it further would not move us forward. As I said before I don't have anything against you personally, only criticising the way you manage these projects which makes it almost impossible to make any changes for potential contributors which results in no progress and I think it's because you don't have time to do everthing you want yet don't let others to do anything you don't think is perfect enough and that results in stalled projects that are almost dead. Also this would scare away all potential contributors who don't want to keep up with your demands. That's why I though it might be good for everyone if you gave the maintainership to someone else which would relieve you from some of your burdens and may allow the project to go somewhere and you would not have to feel responsible for it.
Thank you for the apology, but if a maintainer rejects a patch for valid technical reasons or because the patch introduces a regression is to be expected as part of the review process. Use of targeted attacks and strong language such as "demands" and "perfection" are a reflection of your personal perception of the review process, which I believe is in stark contrast to evidence of the technical discussions contained within the archives.
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.
So then can you plase explain again why this series is not acceptable?
I'm not sure there's much point in repeating myself again, so here is the link from before: https://mail.coreboot.org/hyperkitty/list/openbios@openbios.org/message/5QOG....
I should add that I originally wrote the series as a precursor to a patch that I have waiting here which solves the problem where the NDRV is inadvertently picked up by the ATI VGA device, which was a bug that you previously reported.
Given the length of this thread, unless there are any new revelations then I plan to push this soon.
- 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.
I may eventually come back to that but currently not interested in it.
That's fine with me, I'll review it if/when it appears.
- 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.
I would go through with it and find out if I knew this could be accepted at the end but if the result would be that I submit something then you look at it and say OK but I don't like this so won't be merged that really discourages me to even try it in the first place. So unless you say you're ready to accept such a solution if it works I will only work on this to see if it's possible but that's very low priority. Even now when the series is not yet ready you're already bringing up theoretical problems to justify why you would not take it which does not really help to make progress. Theoretical problems like this next one:
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?
This is a non existing problem for several reasons:
- OpenBIOS does not currently run on real hardware so I doubt you'd test on it.
- It already works differently on QEMU as fw_cfg does not exist elsewhere.
- We already have not 2 but at least 4 different versions just for ppc.
What I propose to chnage is only QEMU and the change would just replace the hard coded device tree fragments currently in arch/ppc/qemu with something passed from QEMU to keep it together with the machines and remove duplicated info from OpenBIOS so they don't have to be kept in sync.
I shall ask you one last time: how will the Forth word bindings work? Until you can provide an outline of this, I am going to ignore this from now on. I should add that there are people who use OpenBIOS and use it on FPGA hardware against QEMU as a reference, so this is a genuine and not theoretical use case.
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.
What I see is that you're trying to make the bar high while in fact it should not be that high to keep everything working. I think it would save us effort because it would make OpenBIOS code much simpler so it should not need that much maintenance.
How much effort would it have saved us over the past couple of years? I'm genuinely curious.
I could see that there could be potential to bring libfdt into OpenBIOS since
I have never proposed to bring libfdt into OpenBIOS and it's not needed. SLOF does not depend on libfdt and it's not needed to unflatten the device tree which can be don fully in Forth. After that point everything works the same as now where we have a device tree constucted in arch/ppc/qemu/init.c. I think you actually don't even get my proposal but you're sure it can't work and you don't want to merge it whatever it may be. That's not a good basis for cooperation.
I do get your proposal, but currently it has gaps and I'm yet to be convinced the benefits outweigh the drawbacks. Whilst you accuse me of not listening to you, how many times have I asked that you *just in this thread* to explain how the FDT to Forth bindings would work? Because that is the key as to whether this approach is feasible or not.
The suggestion of introducing libfdt was considering that if FDTs can be made to work with Forth in a way that isn't inconvenient to the developer, it may be worth standardising its use within OpenBIOS. But for now it's not worth continuing this discussion until you can demonstrate how the key part of the proposal should work.
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.
I did make a proof of concept in OpenFirmware for this where passing the FDT from QEMU and unflattening it worked but I could not make a single OFW ROM work for all PPC machines as OpenFirmware is meant to be for a single machine so I could not put machine support in packages that would only work after machine init. But the interesting part of getting the device tree is done there if you want to have a look. Obviously OpenFirmware has no C so the solution also does not require any changes to the C parts of OpenBIOS. Working with the resulting device tree is the same as now, the only change would be to add a step in startup to construct the initial device tree from the FDT placed in memory by QEMU as it's done in spapr which would replace the hard coded machine info and device tree fragments currently in arch/ppc/qemu so these would not have to be maintained in OpenBIOS and would be together with the machines they describe in QEMU so if something is changed in that machine then the device tre can be updated there and OpenBIOS likely does not need any further changes or maybe only adding a driver if it has to work with the device added. QEMU already depends on libfdt so you can use that to construct the device tree or load it from a file like how sam460ex does for Linux boot.
Yes, I've already seen this.
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 think some people are using fcode-utils to decompile modify and compile patched ROMs for graphics cards for old Macs so this certainly works.
Do you have a link with an example somewhere? I believe romheaders can parse an existing ROM, but I wasn't aware it could create a new one.
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.
Pointing out a real problem helps. Bringing up theoretical problems as a way to justify why this would not work or even if it did work why you would not want to merge it which is what you seem to do most of the time does not help.
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.
Except why this series is not acceptable, which I still don't get as you said I did not understand your answer but that's what I got from your reply so I still don't know why this simple chnage is not appropriate then.
I've reposted the link explaining why I don't think this is a suitable approach above.
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.
I suspect there are no installations of that old NetBSD that could break. Anybody wanting to use NetBSD could likely update to the latest which did work. If there are some users who rely on that and you get reports, you could have just reverted the change. That would have been a more practical approach.
Could they? Real life experience says that this is often not the case. Alternatively as a project we should simply aim to improve functionality without introducing unnecessary regressions for our existing users.
But I don't care about this issue any more. Not sure you've noticed but I've added another machine in QEMU that can run more MorphOS versions than mac99 and also runs AmigaOS quite well which does not support mac99 and now has sound output as well while we're still waiting for screamer to be finished for mac99. So I'm not any more interested in improving mac99. I still have some ideas and may conribute to help people who want to run MacOS but I don't want to fight with you so I'd only consider spending time with it if you also allow changes to be made.
Yes, I've seen the work you've done introducing the new machines into QEMU. AFAICT the only new functionality outstanding for OpenBIOS and mac99 are the patches to allow executing a real ATI FCode ROM, and those have been either already merged or awaiting changes.
ATB,
Mark.