On 18/09/2022 22:17, BALATON Zoltan wrote:
On Sun, 18 Sep 2022, Mark Cave-Ayland wrote:
On 15/09/2022 20:56, BALATON Zoltan wrote:
On Thu, 15 Sep 2022, Mark Cave-Ayland wrote:
On 11/09/2022 11:58, BALATON Zoltan wrote:
Devices that OpenBIOS does not have a driver for may have an FCode driver that would create the open/close methods but this results in duplicate words due to the empty defaults OpenBIOS adds. These empty defaults are added so that open-dev works (which is needed to run an FCode driver) but to avoid this clash change open-dev to check if open method exists before calling it so we can omit the empty defaults and let an FCode driver install them without getting duplicates.
Signed-off-by: BALATON Zoltan balaton@eik.bme.hu
drivers/pci.c | 2 +- forth/device/pathres.fs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/pci.c b/drivers/pci.c index 787fa93..aa8de6b 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -1926,7 +1926,7 @@ static phandle_t ob_configure_pci_device(const char* parent_path, } /* if devices haven't supplied open/close words then supply them with simple defaults */ - if (!find_package_method("open", phandle) && !find_package_method("close", phandle)) { + if (pci_dev && !find_package_method("open", phandle) && !find_package_method("close", phandle)) { BIND_NODE_METHODS(phandle, ob_pci_simple_node); }
I'm fairly sure that I tested the above check for existing "open" and "close" methods when I made the changes to BIND_NODE_METHODS(), so I would already expect the existing code to do the right thing here.
Can you provide a bit more information about your test case? I seem to recall when I wrote it there was no need for modifications to pathres.fs as included below.
I've tried to explain it in the commit message but this happens when a device that has no OpenBIOS driver but has an FCode ROM such as ati-vga,model=rv100 or a SATA controller like sii3112 (which are examples that QEMU can emulate and have MacOS ROMs) are attempted to be passed to MacOS. In this case OpenBIOS creates the device node and issues a "Cannot manage" warning but adds empty open/close methods. Then we run the FCode ROM (by hand as OpenBIOS does not do it by itself) to init the device and set up the device node for MacOS but this also creates open and close methods which then clash with the empty ones added by OpenBIOS. This results in "isn't unique" warnings when the ROM is run and duplicate open and close shown in "words" after the ROM added the correct methods it needs. To avoid this, OpenBIOS should not create these methods for devices it does not manage which the change in drivers/pci.c does, but to run the FCode ROM we need to do " /pci/@x" open-dev to my-self first which then fails as it tries to tun the open method which does not exist now so we need to change open-dev to only call the method if it exists which is the change to pathres does.
I see, thanks. The message about duplicate words is only actually a warning: since the Forth dictionary is a linked list, the most recent definition i.e. that generated by the FCode will always take priority so it won't affect the execution of the FCode in any way.
What we should do here is to add support for FCode contained within PCI option ROMs so that ROMs extracted from real PCI cards could be configured via the QEMU command line, which means the above check for the open and close words in its existing form should work and will also be a fairly trivial patch.
Sure, I'm waiting for you to implement that for 3 years now but the ROMs don't yet run without my patches so the first step would be to merge those at least.
I've provided my feedback on v2 (and I see you've finally addressed some of the review comments from the original patches), so let's see how we go.
ATB,
Mark.