[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c
From: |
Bernhard Beschow |
Subject: |
Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c |
Date: |
Thu, 12 Jan 2023 18:03:11 +0000 |
Am 12. Januar 2023 16:31:23 UTC schrieb "Philippe Mathieu-Daudé"
<philmd@linaro.org>:
>On 12/1/23 16:04, Philippe Mathieu-Daudé wrote:
>> On 9/1/23 18:23, Bernhard Beschow wrote:
>>> Now that the PIIX3 and PIIX4 device models are sufficiently consolidated,
>>> their implementations can be merged into one file for further
>>> consolidation.
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>> Message-Id: <20221022150508.26830-37-shentey@gmail.com>
>>> ---
>>> hw/isa/{piix3.c => piix.c} | 158 ++++++++++++++++++++
>>> hw/isa/piix4.c | 285 -------------------------------------
>>> MAINTAINERS | 6 +-
>>> hw/i386/Kconfig | 2 +-
>>> hw/isa/Kconfig | 12 +-
>>> hw/isa/meson.build | 3 +-
>>> hw/mips/Kconfig | 2 +-
>>> 7 files changed, 165 insertions(+), 303 deletions(-)
>>> rename hw/isa/{piix3.c => piix.c} (75%)
>>> delete mode 100644 hw/isa/piix4.c
>>
>>> +static void piix4_realize(PCIDevice *dev, Error **errp)
>>> +{
>>> + PIIXState *s = PIIX_PCI_DEVICE(dev);
>>> + PCIBus *pci_bus = pci_get_bus(dev);
>>> + ISABus *isa_bus;
>>> +
>>> + isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
>>> + pci_address_space_io(dev), errp);
>>> + if (!isa_bus) {
>>> + return;
>>> + }
>>> +
>>> + memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
>>> + "reset-control", 1);
>>> + memory_region_add_subregion_overlap(pci_address_space_io(dev),
>>> + PIIX_RCR_IOPORT, &s->rcr_mem, 1);
>>> +
>>> + /* initialize i8259 pic */
>>> + if (!qdev_realize(DEVICE(&s->pic), NULL, errp)) {
>>> + return;
>>> + }
>>> +
>>> + /* initialize ISA irqs */
>>> + isa_bus_irqs(isa_bus, s->pic.in_irqs);
>>> +
>>> + /* initialize pit */
>>> + i8254_pit_init(isa_bus, 0x40, 0, NULL);
>>> +
>>> + /* DMA */
>>> + i8257_dma_init(isa_bus, 0);
>>> +
>>> + /* RTC */
>>> + qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
>>> + if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
>>> + return;
>>> + }
>>> + s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
>>
>> Pre-existing; it seems this difference with PIIX3 can be removed
>> because already taken care by calling isa_connect_gpio_out() in
>> mc146818_rtc_init()?
>>
>> ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq
>> intercept_irq)
>> {
>> DeviceState *dev;
>> ISADevice *isadev;
>> RTCState *s;
>>
>> isadev = isa_new(TYPE_MC146818_RTC);
>> dev = DEVICE(isadev);
>> s = MC146818_RTC(isadev);
>> qdev_prop_set_int32(dev, "base_year", base_year);
>> isa_realize_and_unref(isadev, bus, &error_fatal);
>> if (intercept_irq) {
>> qdev_connect_gpio_out(dev, 0, intercept_irq);
>> } else {
>> isa_connect_gpio_out(isadev, 0, s->isairq);
>>
>
>I meant to paste:
>
>static void rtc_realizefn(DeviceState *dev, Error **errp)
>{
> ...
> qdev_init_gpio_out(dev, &s->irq, 1);
>
>
>> Having:
>>
>> void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, unsigned isairq)
>> {
>> qemu_irq irq = isa_get_irq(isadev, isairq);
>> qdev_connect_gpio_out(DEVICE(isadev), gpioirq, irq);
>> }
>>
>> What do you think?
>
In "[PATCH v6 11/33] hw/i386/pc: Create RTC controllers in south bridges"
mc146818_rtc_init() got inlined, taking into account the intercept_irq, which
required the rtc interrupt to be wired up in board code. Since we don't have to
deal with intercept_irq in the Malta code, wiring up of the rtc interrupt could
be moved into PIIX4.
I would prefer to wire up the rtc interrupt in PIIX3 as well, and to re-wire it
in board code in case of intercept_irq != NULL. That's still an open question
which needs to be solved for PIIX4 to become a drop-in replacement for PIIX3.
Any ideas?
Best regards,
Bernhard
- Re: [PATCH v6 22/33] hw/isa/piix3: Drop the "3" from PIIX base class, (continued)
- [PATCH v6 27/33] hw/isa/piix4: Rename reset control operations to match PIIX3, Bernhard Beschow, 2023/01/09
- [PATCH v6 32/33] hw/isa/piix: Consolidate IRQ triggering, Bernhard Beschow, 2023/01/09
- [PATCH v6 33/33] hw/isa/piix: Share PIIX3's base class with PIIX4, Bernhard Beschow, 2023/01/09
- [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c, Bernhard Beschow, 2023/01/09
- Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c, Philippe Mathieu-Daudé, 2023/01/12
- Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c, Philippe Mathieu-Daudé, 2023/01/12
[PATCH v6 26/33] hw/isa/piix4: Reuse struct PIIXState from PIIX3, Bernhard Beschow, 2023/01/09
[PATCH v6 29/33] hw/isa/piix: Harmonize names of reset control memory regions, Bernhard Beschow, 2023/01/09
[PATCH v6 31/33] hw/isa/piix: Rename functions to be shared for interrupt triggering, Bernhard Beschow, 2023/01/09
[PATCH v6 30/33] hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4, Bernhard Beschow, 2023/01/09
Re: [PATCH v6 00/33] Consolidate PIIX south bridges, Philippe Mathieu-Daudé, 2023/01/13