Hi Phil, Am 25. Oktober 2022 23:34:15 UTC schrieb "Philippe Mathieu-Daudé" <phi...@linaro.org>: >On 22/10/22 17:04, Bernhard Beschow wrote: >> PIIX3 initializes the PIRQx route control registers to the default >> values as described in the 82371AB PCI-TO-ISA/IDE XCELERATOR (PIIX4) >> April 1997 manual. PIIX4, however, initializes the routes according to >> the Malta™ User’s Manual, ch 6.6, which are IRQs 10 and 11. In order to >> allow the reset methods to be consolidated, allow board code to specify >> the routes. >> >> Signed-off-by: Bernhard Beschow <shen...@gmail.com> >> --- >> hw/isa/piix3.c | 12 ++++++++---- >> include/hw/southbridge/piix.h | 1 + >> 2 files changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c >> index aa32f43e4a..c6a8f1f27d 100644 >> --- a/hw/isa/piix3.c >> +++ b/hw/isa/piix3.c >> @@ -168,10 +168,10 @@ static void piix3_reset(DeviceState *dev) >> pci_conf[0x4c] = 0x4d; >> pci_conf[0x4e] = 0x03; >> pci_conf[0x4f] = 0x00; >> - pci_conf[0x60] = 0x80; >> - pci_conf[0x61] = 0x80; >> - pci_conf[0x62] = 0x80; >> - pci_conf[0x63] = 0x80; > >These values are the correct reset ones however (also for PIIX4). > >The problem is the Malta machine abuse of the PIIX4 model. IOW >this doesn't seem the correct approach, we should fix how Malta >use the PIIX4 (in the emulated tiny boot loader). I'll try to >write smth before reviewing the rest of this series, because >it might simplify your refactor.
Indeed my first approach for the refactor was to implement MachineClass::reset for Malta where the Malta-specific values would be set. I could redo that if you want. Just let me know. Best regards, Bernhard > >> + pci_conf[PIIX_PIRQCA] = d->pci_irq_reset_mappings[0]; >> + pci_conf[PIIX_PIRQCB] = d->pci_irq_reset_mappings[1]; >> + pci_conf[PIIX_PIRQCC] = d->pci_irq_reset_mappings[2]; >> + pci_conf[PIIX_PIRQCD] = d->pci_irq_reset_mappings[3]; >> pci_conf[0x69] = 0x02; >> pci_conf[0x70] = 0x80; >> pci_conf[0x76] = 0x0c; >> @@ -383,6 +383,10 @@ static void pci_piix3_init(Object *obj) >> static Property pci_piix3_props[] = { >> DEFINE_PROP_UINT32("smb_io_base", PIIX3State, smb_io_base, 0), >> + DEFINE_PROP_UINT8("pirqa", PIIX3State, pci_irq_reset_mappings[0], 0x80), >> + DEFINE_PROP_UINT8("pirqb", PIIX3State, pci_irq_reset_mappings[1], 0x80), >> + DEFINE_PROP_UINT8("pirqc", PIIX3State, pci_irq_reset_mappings[2], 0x80), >> + DEFINE_PROP_UINT8("pirqd", PIIX3State, pci_irq_reset_mappings[3], 0x80), >> DEFINE_PROP_BOOL("has-acpi", PIIX3State, has_acpi, true), >> DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true), >> DEFINE_PROP_BOOL("smm-enabled", PIIX3State, smm_enabled, false), >> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h >> index 1f22eb1444..df3e0084c5 100644 >> --- a/include/hw/southbridge/piix.h >> +++ b/include/hw/southbridge/piix.h >> @@ -54,6 +54,7 @@ struct PIIXState { >> /* This member isn't used. Just for save/load compatibility */ >> int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS]; >> + uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS]; >> ISAPICState pic; >> RTCState rtc; >