|
From: | BALATON Zoltan |
Subject: | Re: [PATCH v3 0/4] ide: implement simple legacy/native mode switching for PCI IDE controllers |
Date: | Sun, 19 Nov 2023 23:11:01 +0100 (CET) |
On Sun, 19 Nov 2023, BALATON Zoltan wrote:
On Thu, 16 Nov 2023, Mark Cave-Ayland wrote:This series adds a simple implementation of legacy/native mode switching for PCIIDE controllers and updates the via-ide device to use it.The approach I take here is to add a new pci_ide_update_mode() function which handles management of the PCI BARs and legacy IDE ioports for each mode to avoid exposingdetails of the internal logic to individual PCI IDE controllers.As noted in [1] this is extracted from a local WIP branch I have which containsfurther work in this area. However for the moment I've kept it simple (and restricted it to the via-ide device) which is good enough for Zoltan's PPC images whilst paving the way for future improvements after 8.2. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [1] https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg05403.html v3: - Rebase onto master- Move ide_portio_list[] and ide_portio_list2[] to IDE core to prevent duplication inhw/ide/pci.c- Don't zero BARs when switching from native mode to legacy mode, instead always force them to read zero as suggested in the PCI IDE specification (note: this also appearsto fix the fuloong2e machine booting from IDE)Not sure you're getting this, see also: https://lists.nongnu.org/archive/html/qemu-devel/2023-11/msg04167.htmlbut this seems to break latest version of the AmigaOS driver for some reason. I assume this is the BAR zeroing that causes this as it works with v2 series and nothing else changed in v3 that could cause this. Testing was done by Rene Engel, cc'd so maybe he can add more info. It seems to work with my patch that sets BARs to legacy values and with v2 that sets them to 0 but not with v3 which should also read 0 but maybe something is off here.
The change log of the AmigaOS driver version which only works with v2 of this series says:
"Replaced ReadConfigLong(PCI_BASE_ADDRESS_x) calls with their PCI interface variant GetResourceRange(x)->BaseAddress to get the correct base address instead of the *bus* base address."but not sure what that means. Apparently it reads BAR values differently and crashes before finding the IDE ports with v3 but not with v2. As it crashes it does not even print debug logs to check what it read.
(Additionally would also need the default value for BMDMA BAR4 as I wrote before but that's disabled by default so only an issue if one tries to enable it. Would be nics to have though as UDMA is faster.)
Regards, BALATON Zoltan
- Add comments in pci_ide_update_mode() suggested by Kevin- Drop the existing R-B and T-B tags: whilst this passes my local tests, the behaviouraround zero BARs feels different enough here v2: - Rebase onto master- Mask the bottom 4 bits of PCI_CLASS_PROG in pci_ide_update_mode() in patch 1- Add patch 2 to remove the default BAR addresses to avoid confusion- Don't set PCI_INTERRUPT_PIN directly in via_ide_reset() as it is already set by pci_ide_update_mode() in patch 3, and reword the commit message accordingly- Add Tested-By tags from Zoltan and Bernhard Mark Cave-Ayland (4): ide/ioport: move ide_portio_list[] and ide_portio_list2[] definitions to IDE core ide/pci: introduce pci_ide_update_mode() function ide/via: don't attempt to set default BAR addresses hw/ide/via: implement legacy/native mode switching hw/ide/core.c | 12 ++++++ hw/ide/ioport.c | 12 ------ hw/ide/pci.c | 84 +++++++++++++++++++++++++++++++++++++++ hw/ide/via.c | 44 ++++++++++++++++---- include/hw/ide/internal.h | 3 ++ include/hw/ide/pci.h | 1 + 6 files changed, 137 insertions(+), 19 deletions(-)
[Prev in Thread] | Current Thread | [Next in Thread] |