| Message ID | 20221022150508.26830-20-shentey@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Consolidate PIIX south bridges | expand |
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 <shentey@gmail.com> > --- > hw/isa/piix3.c | 12 ++++++++---- > include/hw/southbridge/piix.h | 1 + > 2 files changed, 9 insertions(+), 4 deletions(-) > 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]; pci_irq_reset_mappings[PCI_NUM_PINS]? > > ISAPICState pic; > RTCState rtc;
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 <shentey@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. > + 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;
Hi Phil, Am 25. Oktober 2022 23:34:15 UTC schrieb "Philippe Mathieu-Daudé" <philmd@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 <shentey@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; >
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; + 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;
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 <shentey@gmail.com> --- hw/isa/piix3.c | 12 ++++++++---- include/hw/southbridge/piix.h | 1 + 2 files changed, 9 insertions(+), 4 deletions(-)