Re: [Qemu-devel] [PATCH 02/17] pseries: rework XICS

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 07/09/2013 11:58 PM, Anthony Liguori wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> 
>> On Tue, 2013-07-09 at 13:40 +1000, Alexey Kardashevskiy wrote:
>>> No, why? It is a per CPU state of XICS controller, never exists apart
>>> from XICS.
>>
>> ICP is. ICS is  ... different but can mostly be considered to be the
>> XICS itself.
>>
>> Anthony, we could be completely anal about it and create a gigantic
>> cathedral of devices or just be a bit realistic and do something simpler
>> that has the exact same functionality :)
> 
> There's very little complexity in making something a device.  It's just
> a matter of sticking a DeviceState member in the struct, changing the
> way the object is created (object_new vs. malloc), and adding a
> TypeInfo.
> 
> There's a very good reason to have things be devices too.  You can only
> control the section naming of devices for live migration.  The only way
> to set compatibility properties for live migration is by having device
> properties too.
> 
> You haven't dealt with these problems yet, but you will, and doing the
> work up front means that you don't have to break migration once in order
> to keep it compatible in the future.



I have got a problem right now. I need to have 2 devices - xics and
xics-kvm, give them the same VMState properties and migration names and
have different pre_load/post_load (btw the whole point of separating
xics-kvm from xics).

How do I solve this without anyone saying that I am doing terribly a wrong
thing?

I already asked this in "[Qemu-devel] [PATCH 05/17] pseries: savevm support
for XICS interrupt controller" but have not seen any response yet. Thank you.




>> Basically, in HW the layout of the interrupt network is:
>>
>>  - One ICP per processor thread (the "presenter"). This contains the
>> registers to fetch a pending interrupt (ack), EOI, and control the
>> processor priority.
>>
>>  - One ICS per logical source of interrupts (ie, one per PCI host
>> bridge, and a few others here or there). This contains the per-interrupt
>> source configuration (target processor(s), priority, mask) and the
>> per-interrupt internal state.
> 
> This sounds an awful lot like the relationship between the I/O APIC(s)
> and the local APICs FWIW.
> 
>> Under PAPR, there is a single "virtual" ICS ... somewhat (it's a bit
>> oddball what pHyp does here, arguably there are two but we can ignore
>> that distinction). There is no register level access. A pair of firmware
>> (RTAS) calls is used to configure each virtual interrupt.
>>
>> So our model here is somewhat the same. We have one ICS in the emulated
>> XICS which arguably *is* the emulated XICS, there's no point making it a
>> separate "device", that would just be gross, and each VCPU has an
>> associated ICP.
> 
> There's nothing gross about making the things that are devices devices.


-- 
Alexey