[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 08/13] vt82c686: Move creation of ISA devices to the ISA b
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v2 08/13] vt82c686: Move creation of ISA devices to the ISA bridge |
Date: |
Sun, 10 Jan 2021 12:34:14 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
+PCI experts
On 1/10/21 1:43 AM, BALATON Zoltan wrote:
> On Sun, 10 Jan 2021, Philippe Mathieu-Daudé wrote:
>> Hi Zoltan,
>>
>> On 1/9/21 9:16 PM, BALATON Zoltan wrote:
>>> Currently the ISA devices that are part of the VIA south bridge,
>>> superio chip are wired up by board code. Move creation of these ISA
>>> devices to the VIA ISA bridge model so that board code does not need
>>> to access ISA bus. This also allows vt82c686b-superio to be made
>>> internal to vt82c686 which allows implementing its configuration via
>>> registers in subseqent commits.
>>
>> Is this patch dependent of the VT82C686B_PM changes
>> or can it be applied before them?
>
> I don't know but why would that be better? I thought it's clearer to
> clean up pm related parts first before moving more stuff to this file so
> that's why this patch comes after (and also because that's the order I
> did it).
Not any better, but easier for me to get your patches integrated,
as I'm reviewing your patches slowly. Finding other reviewers
would certainly help.
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>> hw/isa/vt82c686.c | 20 ++++++++++++++++++++
>>> hw/mips/fuloong2e.c | 29 +++++------------------------
>>> 2 files changed, 25 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>> index 58c0bba1d0..5df9be8ff4 100644
>>> --- a/hw/isa/vt82c686.c
>>> +++ b/hw/isa/vt82c686.c
>>> @@ -16,6 +16,11 @@
>>> #include "hw/qdev-properties.h"
>>> #include "hw/isa/isa.h"
>>> #include "hw/isa/superio.h"
>>> +#include "hw/intc/i8259.h"
>>> +#include "hw/irq.h"
>>> +#include "hw/dma/i8257.h"
>>> +#include "hw/timer/i8254.h"
>>> +#include "hw/rtc/mc146818rtc.h"
>>> #include "migration/vmstate.h"
>>> #include "hw/isa/apm.h"
>>> #include "hw/acpi/acpi.h"
>>> @@ -307,9 +312,16 @@ OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState,
>>> VT82C686B_ISA)
>>>
>>> struct VT82C686BISAState {
>>> PCIDevice dev;
>>> + qemu_irq cpu_intr;
>>> SuperIOConfig superio_cfg;
>>> };
>>>
>>> +static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
>>> +{
>>> + VT82C686BISAState *s = opaque;
>>> + qemu_set_irq(s->cpu_intr, level);
>>> +}
>>> +
>>> static void vt82c686b_write_config(PCIDevice *d, uint32_t addr,
>>> uint32_t val, int len)
>>> {
>>> @@ -365,10 +377,18 @@ static void vt82c686b_realize(PCIDevice *d,
>>> Error **errp)
>>> VT82C686BISAState *s = VT82C686B_ISA(d);
>>> DeviceState *dev = DEVICE(d);
>>> ISABus *isa_bus;
>>> + qemu_irq *isa_irq;
>>> int i;
>>>
>>> + qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>>
>> Why not use the SysBus API?
>
> How? This is a PCIDevice not a SysBusDevice.
Indeed :)
>>> + isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
>>> isa_bus = isa_bus_new(dev, get_system_memory(),
>>> pci_address_space_io(d),
>>> &error_fatal);
>>
>> Isn't it get_system_memory() -> pci_address_space(d)?
>
> I don't really know. Most other places that create an isa bus seem to
> also use get_system_memory(), only piix4 uses pci_address_space(dev) so
> I thought if those others are OK this should be too.
I'm not a PCI expert but my understanding is PCI device functions are
restricted to the PCI bus address space. The host bridge may map this
space within the host.
QEMU might be using get_system_memory() because for some host bridge
the mapping is not implemented so it was easier this way?
Regards,
Phil.
- [PATCH v2 12/13] vt82c686: Add VT8231_SUPERIO based on VIA_SUPERIO, (continued)
- [PATCH v2 12/13] vt82c686: Add VT8231_SUPERIO based on VIA_SUPERIO, BALATON Zoltan, 2021/01/09
- [PATCH v2 02/13] vt82c686: Reorganise code, BALATON Zoltan, 2021/01/09
- [PATCH v2 07/13] vt82c686: Simplify vt82c686b_realize(), BALATON Zoltan, 2021/01/09
- [PATCH v2 05/13] vt82c686: Set user_creatable=false for VT82C686B_PM, BALATON Zoltan, 2021/01/09
- [PATCH v2 13/13] vt82c686: Add emulation of VT8231 south bridge, BALATON Zoltan, 2021/01/09
- [PATCH v2 08/13] vt82c686: Move creation of ISA devices to the ISA bridge, BALATON Zoltan, 2021/01/09
- Re: [PATCH v2 08/13] vt82c686: Move creation of ISA devices to the ISA bridge, Philippe Mathieu-Daudé, 2021/01/09
- Re: [PATCH v2 08/13] vt82c686: Move creation of ISA devices to the ISA bridge, BALATON Zoltan, 2021/01/09
- Re: [PATCH v2 08/13] vt82c686: Move creation of ISA devices to the ISA bridge,
Philippe Mathieu-Daudé <=
- Re: [PATCH v2 08/13] vt82c686: Move creation of ISA devices to the ISA bridge, BALATON Zoltan, 2021/01/10
- Re: [PATCH v2 08/13] vt82c686: Move creation of ISA devices to the ISA bridge, Jiaxun Yang, 2021/01/10
- Re: [PATCH v2 08/13] vt82c686: Move creation of ISA devices to the ISA bridge, BALATON Zoltan, 2021/01/11
- Re: [PATCH v2 08/13] vt82c686: Move creation of ISA devices to the ISA bridge, Philippe Mathieu-Daudé, 2021/01/25
[PATCH v2 06/13] vt82c686: Make vt82c686b-pm an abstract base class and add vt8231-pm based on it, BALATON Zoltan, 2021/01/09
[PATCH v2 10/13] vt82c686: Implement control of serial port io ranges via config regs, BALATON Zoltan, 2021/01/09
[PATCH v2 09/13] vt82c686: Fix superio_cfg_{read,write}() functions, BALATON Zoltan, 2021/01/09
[PATCH v2 11/13] vt82c686: QOM-ify superio related functionality, BALATON Zoltan, 2021/01/09