| Message ID | 0072079efad1c31da849cff7ad2cb426aeb6c29f.1296744934.git.jan.kiszka@siemens.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > The registers of real IOAPICs can be relocated during runtime (via > chipset registers). We don't support this yet, but qemu-kvm carries the > current base address in its version 2 vmstate. > > To align both implementations for migratability, add the proper > infrastructure to accept initial as well as updated base addresses and > include the current address in the vmstate. This is done in a way that > will also allow multiple IOAPICs in the future. Nack, the addresses should be device properties.
On 2011-02-03 18:03, Blue Swirl wrote: > On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> The registers of real IOAPICs can be relocated during runtime (via >> chipset registers). We don't support this yet, but qemu-kvm carries the >> current base address in its version 2 vmstate. >> >> To align both implementations for migratability, add the proper >> infrastructure to accept initial as well as updated base addresses and >> include the current address in the vmstate. This is done in a way that >> will also allow multiple IOAPICs in the future. > > Nack, the addresses should be device properties. Hmm.... we could make default_base_address a property. Will change that. But current_base_address is just the same as apicbase and can't be a property. Jan
On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2011-02-03 18:03, Blue Swirl wrote: >> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>> The registers of real IOAPICs can be relocated during runtime (via >>> chipset registers). We don't support this yet, but qemu-kvm carries the >>> current base address in its version 2 vmstate. >>> >>> To align both implementations for migratability, add the proper >>> infrastructure to accept initial as well as updated base addresses and >>> include the current address in the vmstate. This is done in a way that >>> will also allow multiple IOAPICs in the future. >> >> Nack, the addresses should be device properties. > > Hmm.... we could make default_base_address a property. Will change that. > But current_base_address is just the same as apicbase and can't be a > property. Oh, right. What will current_base_address used for? Why can't board just unmap IOAPIC from current address and remap it at the new address? Then the device would not need to know its base address.
On 2011-02-03 18:36, Blue Swirl wrote: > On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> On 2011-02-03 18:03, Blue Swirl wrote: >>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>> The registers of real IOAPICs can be relocated during runtime (via >>>> chipset registers). We don't support this yet, but qemu-kvm carries the >>>> current base address in its version 2 vmstate. >>>> >>>> To align both implementations for migratability, add the proper >>>> infrastructure to accept initial as well as updated base addresses and >>>> include the current address in the vmstate. This is done in a way that >>>> will also allow multiple IOAPICs in the future. >>> >>> Nack, the addresses should be device properties. >> >> Hmm.... we could make default_base_address a property. Will change that. >> But current_base_address is just the same as apicbase and can't be a >> property. > > Oh, right. What will current_base_address used for? Why can't board > just unmap IOAPIC from current address and remap it at the new > address? Then the device would not need to know its base address. The board could do this. The question is where we put this service, in the context if the IOAPIC as ioapic_set_base_address (compare to cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into each and every board code. In the latter case, the boards would also be responsible for saving/restoring the address. Well, unless we have a good reason, I would prefer to keep the split as suggested, also to avoid that qemu-kvm needs to break its vmstate. Jan PS: Another bug in the APIC emulation: cpu_set_apic_base lacks a call to sysbus_mmio_map to update its mapping.
On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2011-02-03 18:36, Blue Swirl wrote: >> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>> On 2011-02-03 18:03, Blue Swirl wrote: >>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>> The registers of real IOAPICs can be relocated during runtime (via >>>>> chipset registers). We don't support this yet, but qemu-kvm carries the >>>>> current base address in its version 2 vmstate. >>>>> >>>>> To align both implementations for migratability, add the proper >>>>> infrastructure to accept initial as well as updated base addresses and >>>>> include the current address in the vmstate. This is done in a way that >>>>> will also allow multiple IOAPICs in the future. >>>> >>>> Nack, the addresses should be device properties. >>> >>> Hmm.... we could make default_base_address a property. Will change that. >>> But current_base_address is just the same as apicbase and can't be a >>> property. >> >> Oh, right. What will current_base_address used for? Why can't board >> just unmap IOAPIC from current address and remap it at the new >> address? Then the device would not need to know its base address. > > The board could do this. The question is where we put this service, in > the context if the IOAPIC as ioapic_set_base_address (compare to > cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into > each and every board code. In the latter case, the boards would also be > responsible for saving/restoring the address. How is the device relocated? Where are the chipset registers you mention?
On 2011-02-03 18:54, Blue Swirl wrote: > On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> On 2011-02-03 18:36, Blue Swirl wrote: >>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>> On 2011-02-03 18:03, Blue Swirl wrote: >>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>> The registers of real IOAPICs can be relocated during runtime (via >>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the >>>>>> current base address in its version 2 vmstate. >>>>>> >>>>>> To align both implementations for migratability, add the proper >>>>>> infrastructure to accept initial as well as updated base addresses and >>>>>> include the current address in the vmstate. This is done in a way that >>>>>> will also allow multiple IOAPICs in the future. >>>>> >>>>> Nack, the addresses should be device properties. >>>> >>>> Hmm.... we could make default_base_address a property. Will change that. >>>> But current_base_address is just the same as apicbase and can't be a >>>> property. >>> >>> Oh, right. What will current_base_address used for? Why can't board >>> just unmap IOAPIC from current address and remap it at the new >>> address? Then the device would not need to know its base address. >> >> The board could do this. The question is where we put this service, in >> the context if the IOAPIC as ioapic_set_base_address (compare to >> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into >> each and every board code. In the latter case, the boards would also be >> responsible for saving/restoring the address. > > How is the device relocated? Where are the chipset registers you mention? Intel's PIIX chipsets contain a register called APICBASE (but it means the IOAPIC), and that defines the location. The analogy in the APIC world is the MSR_IA32_APICBASE which we maintain via the APIC state. Jan
On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2011-02-03 18:54, Blue Swirl wrote: >> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>> On 2011-02-03 18:36, Blue Swirl wrote: >>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>> On 2011-02-03 18:03, Blue Swirl wrote: >>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>> The registers of real IOAPICs can be relocated during runtime (via >>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the >>>>>>> current base address in its version 2 vmstate. >>>>>>> >>>>>>> To align both implementations for migratability, add the proper >>>>>>> infrastructure to accept initial as well as updated base addresses and >>>>>>> include the current address in the vmstate. This is done in a way that >>>>>>> will also allow multiple IOAPICs in the future. >>>>>> >>>>>> Nack, the addresses should be device properties. >>>>> >>>>> Hmm.... we could make default_base_address a property. Will change that. >>>>> But current_base_address is just the same as apicbase and can't be a >>>>> property. >>>> >>>> Oh, right. What will current_base_address used for? Why can't board >>>> just unmap IOAPIC from current address and remap it at the new >>>> address? Then the device would not need to know its base address. >>> >>> The board could do this. The question is where we put this service, in >>> the context if the IOAPIC as ioapic_set_base_address (compare to >>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into >>> each and every board code. In the latter case, the boards would also be >>> responsible for saving/restoring the address. >> >> How is the device relocated? Where are the chipset registers you mention? > > Intel's PIIX chipsets contain a register called APICBASE (but it means > the IOAPIC), and that defines the location. The analogy in the APIC > world is the MSR_IA32_APICBASE which we maintain via the APIC state. In ICH10 the register is called OIC—Other Interrupt Control Register and the interesting bits APIC Range Select (ASEL). So actually PIIX should manage IOAPIC mapping, not board level.
On 2011-02-03 20:01, Blue Swirl wrote: > On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> On 2011-02-03 18:54, Blue Swirl wrote: >>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>> On 2011-02-03 18:36, Blue Swirl wrote: >>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>> On 2011-02-03 18:03, Blue Swirl wrote: >>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>>> The registers of real IOAPICs can be relocated during runtime (via >>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the >>>>>>>> current base address in its version 2 vmstate. >>>>>>>> >>>>>>>> To align both implementations for migratability, add the proper >>>>>>>> infrastructure to accept initial as well as updated base addresses and >>>>>>>> include the current address in the vmstate. This is done in a way that >>>>>>>> will also allow multiple IOAPICs in the future. >>>>>>> >>>>>>> Nack, the addresses should be device properties. >>>>>> >>>>>> Hmm.... we could make default_base_address a property. Will change that. >>>>>> But current_base_address is just the same as apicbase and can't be a >>>>>> property. >>>>> >>>>> Oh, right. What will current_base_address used for? Why can't board >>>>> just unmap IOAPIC from current address and remap it at the new >>>>> address? Then the device would not need to know its base address. >>>> >>>> The board could do this. The question is where we put this service, in >>>> the context if the IOAPIC as ioapic_set_base_address (compare to >>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into >>>> each and every board code. In the latter case, the boards would also be >>>> responsible for saving/restoring the address. >>> >>> How is the device relocated? Where are the chipset registers you mention? >> >> Intel's PIIX chipsets contain a register called APICBASE (but it means >> the IOAPIC), and that defines the location. The analogy in the APIC >> world is the MSR_IA32_APICBASE which we maintain via the APIC state. > > In ICH10 the register is called OIC—Other Interrupt Control Register > and the interesting bits APIC Range Select (ASEL). > > So actually PIIX should manage IOAPIC mapping, not board level. The point is we need ioapic_set_base_address logic in multiple places (once chipsets start to implement it). Better push it to a central place from the beginning. Also the bit keeping. There is no difference to apicbase. Jan
On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2011-02-03 20:01, Blue Swirl wrote: >> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>> On 2011-02-03 18:54, Blue Swirl wrote: >>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>> On 2011-02-03 18:36, Blue Swirl wrote: >>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>> On 2011-02-03 18:03, Blue Swirl wrote: >>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via >>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the >>>>>>>>> current base address in its version 2 vmstate. >>>>>>>>> >>>>>>>>> To align both implementations for migratability, add the proper >>>>>>>>> infrastructure to accept initial as well as updated base addresses and >>>>>>>>> include the current address in the vmstate. This is done in a way that >>>>>>>>> will also allow multiple IOAPICs in the future. >>>>>>>> >>>>>>>> Nack, the addresses should be device properties. >>>>>>> >>>>>>> Hmm.... we could make default_base_address a property. Will change that. >>>>>>> But current_base_address is just the same as apicbase and can't be a >>>>>>> property. >>>>>> >>>>>> Oh, right. What will current_base_address used for? Why can't board >>>>>> just unmap IOAPIC from current address and remap it at the new >>>>>> address? Then the device would not need to know its base address. >>>>> >>>>> The board could do this. The question is where we put this service, in >>>>> the context if the IOAPIC as ioapic_set_base_address (compare to >>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into >>>>> each and every board code. In the latter case, the boards would also be >>>>> responsible for saving/restoring the address. >>>> >>>> How is the device relocated? Where are the chipset registers you mention? >>> >>> Intel's PIIX chipsets contain a register called APICBASE (but it means >>> the IOAPIC), and that defines the location. The analogy in the APIC >>> world is the MSR_IA32_APICBASE which we maintain via the APIC state. >> >> In ICH10 the register is called OIC—Other Interrupt Control Register >> and the interesting bits APIC Range Select (ASEL). >> >> So actually PIIX should manage IOAPIC mapping, not board level. > > The point is we need ioapic_set_base_address logic in multiple places > (once chipsets start to implement it). Better push it to a central place > from the beginning. Also the bit keeping. There is no difference to > apicbase. In that case, the function should be made inline version in ioapic.h.
On 2011-02-03 20:11, Blue Swirl wrote: > On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> On 2011-02-03 20:01, Blue Swirl wrote: >>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>> On 2011-02-03 18:54, Blue Swirl wrote: >>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>> On 2011-02-03 18:36, Blue Swirl wrote: >>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote: >>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via >>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the >>>>>>>>>> current base address in its version 2 vmstate. >>>>>>>>>> >>>>>>>>>> To align both implementations for migratability, add the proper >>>>>>>>>> infrastructure to accept initial as well as updated base addresses and >>>>>>>>>> include the current address in the vmstate. This is done in a way that >>>>>>>>>> will also allow multiple IOAPICs in the future. >>>>>>>>> >>>>>>>>> Nack, the addresses should be device properties. >>>>>>>> >>>>>>>> Hmm.... we could make default_base_address a property. Will change that. >>>>>>>> But current_base_address is just the same as apicbase and can't be a >>>>>>>> property. >>>>>>> >>>>>>> Oh, right. What will current_base_address used for? Why can't board >>>>>>> just unmap IOAPIC from current address and remap it at the new >>>>>>> address? Then the device would not need to know its base address. >>>>>> >>>>>> The board could do this. The question is where we put this service, in >>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to >>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into >>>>>> each and every board code. In the latter case, the boards would also be >>>>>> responsible for saving/restoring the address. >>>>> >>>>> How is the device relocated? Where are the chipset registers you mention? >>>> >>>> Intel's PIIX chipsets contain a register called APICBASE (but it means >>>> the IOAPIC), and that defines the location. The analogy in the APIC >>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state. >>> >>> In ICH10 the register is called OIC—Other Interrupt Control Register >>> and the interesting bits APIC Range Select (ASEL). >>> >>> So actually PIIX should manage IOAPIC mapping, not board level. >> >> The point is we need ioapic_set_base_address logic in multiple places >> (once chipsets start to implement it). Better push it to a central place >> from the beginning. Also the bit keeping. There is no difference to >> apicbase. > > In that case, the function should be made inline version in ioapic.h. That still replicates the bit keeping. I don't see the benefit of moving it over, even less when we want to consolidate with a vmstate layout that is already in use. Jan
On Thu, Feb 3, 2011 at 7:25 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2011-02-03 20:11, Blue Swirl wrote: >> On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>> On 2011-02-03 20:01, Blue Swirl wrote: >>>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>> On 2011-02-03 18:54, Blue Swirl wrote: >>>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>> On 2011-02-03 18:36, Blue Swirl wrote: >>>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote: >>>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via >>>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the >>>>>>>>>>> current base address in its version 2 vmstate. >>>>>>>>>>> >>>>>>>>>>> To align both implementations for migratability, add the proper >>>>>>>>>>> infrastructure to accept initial as well as updated base addresses and >>>>>>>>>>> include the current address in the vmstate. This is done in a way that >>>>>>>>>>> will also allow multiple IOAPICs in the future. >>>>>>>>>> >>>>>>>>>> Nack, the addresses should be device properties. >>>>>>>>> >>>>>>>>> Hmm.... we could make default_base_address a property. Will change that. >>>>>>>>> But current_base_address is just the same as apicbase and can't be a >>>>>>>>> property. >>>>>>>> >>>>>>>> Oh, right. What will current_base_address used for? Why can't board >>>>>>>> just unmap IOAPIC from current address and remap it at the new >>>>>>>> address? Then the device would not need to know its base address. >>>>>>> >>>>>>> The board could do this. The question is where we put this service, in >>>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to >>>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into >>>>>>> each and every board code. In the latter case, the boards would also be >>>>>>> responsible for saving/restoring the address. >>>>>> >>>>>> How is the device relocated? Where are the chipset registers you mention? >>>>> >>>>> Intel's PIIX chipsets contain a register called APICBASE (but it means >>>>> the IOAPIC), and that defines the location. The analogy in the APIC >>>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state. >>>> >>>> In ICH10 the register is called OIC—Other Interrupt Control Register >>>> and the interesting bits APIC Range Select (ASEL). >>>> >>>> So actually PIIX should manage IOAPIC mapping, not board level. >>> >>> The point is we need ioapic_set_base_address logic in multiple places >>> (once chipsets start to implement it). Better push it to a central place >>> from the beginning. Also the bit keeping. There is no difference to >>> apicbase. >> >> In that case, the function should be made inline version in ioapic.h. > > That still replicates the bit keeping. > > I don't see the benefit of moving it over, even less when we want to > consolidate with a vmstate layout that is already in use. The benefit is that the device model is improved.
On 2011-02-03 20:30, Blue Swirl wrote: > On Thu, Feb 3, 2011 at 7:25 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> On 2011-02-03 20:11, Blue Swirl wrote: >>> On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>> On 2011-02-03 20:01, Blue Swirl wrote: >>>>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>> On 2011-02-03 18:54, Blue Swirl wrote: >>>>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>>> On 2011-02-03 18:36, Blue Swirl wrote: >>>>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote: >>>>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via >>>>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the >>>>>>>>>>>> current base address in its version 2 vmstate. >>>>>>>>>>>> >>>>>>>>>>>> To align both implementations for migratability, add the proper >>>>>>>>>>>> infrastructure to accept initial as well as updated base addresses and >>>>>>>>>>>> include the current address in the vmstate. This is done in a way that >>>>>>>>>>>> will also allow multiple IOAPICs in the future. >>>>>>>>>>> >>>>>>>>>>> Nack, the addresses should be device properties. >>>>>>>>>> >>>>>>>>>> Hmm.... we could make default_base_address a property. Will change that. >>>>>>>>>> But current_base_address is just the same as apicbase and can't be a >>>>>>>>>> property. >>>>>>>>> >>>>>>>>> Oh, right. What will current_base_address used for? Why can't board >>>>>>>>> just unmap IOAPIC from current address and remap it at the new >>>>>>>>> address? Then the device would not need to know its base address. >>>>>>>> >>>>>>>> The board could do this. The question is where we put this service, in >>>>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to >>>>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into >>>>>>>> each and every board code. In the latter case, the boards would also be >>>>>>>> responsible for saving/restoring the address. >>>>>>> >>>>>>> How is the device relocated? Where are the chipset registers you mention? >>>>>> >>>>>> Intel's PIIX chipsets contain a register called APICBASE (but it means >>>>>> the IOAPIC), and that defines the location. The analogy in the APIC >>>>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state. >>>>> >>>>> In ICH10 the register is called OIC—Other Interrupt Control Register >>>>> and the interesting bits APIC Range Select (ASEL). >>>>> >>>>> So actually PIIX should manage IOAPIC mapping, not board level. >>>> >>>> The point is we need ioapic_set_base_address logic in multiple places >>>> (once chipsets start to implement it). Better push it to a central place >>>> from the beginning. Also the bit keeping. There is no difference to >>>> apicbase. >>> >>> In that case, the function should be made inline version in ioapic.h. >> >> That still replicates the bit keeping. >> >> I don't see the benefit of moving it over, even less when we want to >> consolidate with a vmstate layout that is already in use. > > The benefit is that the device model is improved. I disagree about the benefit, but I will simply drop this patch and instead add a dummy field to a vmstate version 3 so that qemu-kvm can remove the base_address evaluation logic when updating. Jan
diff --git a/hw/ioapic.c b/hw/ioapic.c index c7019f5..b53d436 100644 --- a/hw/ioapic.c +++ b/hw/ioapic.c @@ -63,6 +63,8 @@ struct IOAPICState { uint32_t irr; uint64_t ioredtbl[IOAPIC_NUM_PINS]; + uint64_t default_base_address; + uint64_t current_base_address; }; static IOAPICState *ioapics[MAX_IOAPICS]; @@ -237,6 +239,7 @@ static int ioapic_pre_load(void *opaque) /* in case we are loading version 1, set these to sane values */ s->irr = 0; + s->current_base_address = s->default_base_address; return 0; } @@ -249,6 +252,7 @@ static const VMStateDescription vmstate_ioapic = { .fields = (VMStateField []) { VMSTATE_UINT8(id, IOAPICState), VMSTATE_UINT8(ioregsel, IOAPICState), + VMSTATE_UINT64_V(current_base_address, IOAPICState, 2), VMSTATE_UINT32_V(irr, IOAPICState, 2), VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS), VMSTATE_END_OF_LIST() @@ -260,6 +264,8 @@ static void ioapic_reset(DeviceState *d) IOAPICState *s = DO_UPCAST(IOAPICState, busdev.qdev, d); int i; + s->current_base_address = s->default_base_address; + sysbus_mmio_map(&s->busdev, 0, s->current_base_address); s->id = 0; s->ioregsel = 0; s->irr = 0; @@ -279,6 +285,17 @@ static CPUWriteMemoryFunc * const ioapic_mem_write[3] = { ioapic_mem_writel, }; +void ioapic_set_base_address(DeviceState *d, target_phys_addr_t addr) +{ + IOAPICState *s = DO_UPCAST(IOAPICState, busdev.qdev, d); + + s->current_base_address = addr; + if (!s->default_base_address) { + s->default_base_address = addr; + } + sysbus_mmio_map(&s->busdev, 0, addr); +} + static int ioapic_init1(SysBusDevice *dev) { IOAPICState *s = FROM_SYSBUS(IOAPICState, dev); diff --git a/hw/ioapic.h b/hw/ioapic.h index c441567..1130d60 100644 --- a/hw/ioapic.h +++ b/hw/ioapic.h @@ -17,4 +17,8 @@ * License along with this library; if not, see <http://www.gnu.org/licenses/>. */ +#define IOAPIC_DEFAULT_BASE_ADDRESS 0xfec00000 + void ioapic_eio_broadcast(int vector); + +void ioapic_set_base_address(DeviceState *d, target_phys_addr_t addr); diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 7b74473..479334f 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -25,6 +25,7 @@ #include "hw.h" #include "pc.h" #include "apic.h" +#include "ioapic.h" #include "pci.h" #include "usb-uhci.h" #include "usb-ohci.h" @@ -46,13 +47,11 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 }; static void ioapic_init(IsaIrqState *isa_irq_state) { DeviceState *dev; - SysBusDevice *d; unsigned int i; dev = qdev_create(NULL, "ioapic"); qdev_init_nofail(dev); - d = sysbus_from_qdev(dev); - sysbus_mmio_map(d, 0, 0xfec00000); + ioapic_set_base_address(dev, IOAPIC_DEFAULT_BASE_ADDRESS); for (i = 0; i < IOAPIC_NUM_PINS; i++) { isa_irq_state->ioapic[i] = qdev_get_gpio_in(dev, i);
The registers of real IOAPICs can be relocated during runtime (via chipset registers). We don't support this yet, but qemu-kvm carries the current base address in its version 2 vmstate. To align both implementations for migratability, add the proper infrastructure to accept initial as well as updated base addresses and include the current address in the vmstate. This is done in a way that will also allow multiple IOAPICs in the future. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/ioapic.c | 17 +++++++++++++++++ hw/ioapic.h | 4 ++++ hw/pc_piix.c | 5 ++--- 3 files changed, 23 insertions(+), 3 deletions(-)