Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

[maobibo]

在 2022/5/10 10:54, maobibo 写道:
> 
> 
> 在 2022/5/10 02:25, Richard Henderson 写道:
>> On 5/9/22 13:04, Peter Maydell wrote:
>>> On Mon, 9 May 2022 at 18:56, Richard Henderson
>>> <richard.henderson@linaro.org> wrote:
>>>> I'm not 100% sure how this "Other configuration control register" should 
>>>> be handled, but
>>>> definitely not like this.
>>>>
>>>> I see you're putting control of this register into loongarch_qemu_read in
>>>> target/loongarch/cpu.c.  Which, I suppose is fair, because this is 
>>>> documented as part of
>>>> the 3A5000 cpu documentation.  But then you split out all of the devices 
>>>> which are *also*
>>>> documented as part of the cpu into the board configuration.
>>>>
>>>> This reminds me of the memory-mapped interface that the armv7m cpu has 
>>>> with its own
>>>> registers.  I believe that you need to model this similarly, where you 
>>>> will have a device
>>>> that represents the cpu, and then instantiates all of the devices that are 
>>>> listed in the
>>>> Loongson 3A5000 TRM -- call this ls3a to match the ls7a name you have for 
>>>> the 7A1000
>>>> bridge device.
>>>>
>>>> When there is a write to the ls3a "Other function configuration register", 
>>>> the ls3a will
>>>> need to communicate the changes to the various bits to its various 
>>>> sub-devices.  I do not
>>>> think it unreasonable to have direct function calls between the components.
>>>>
>>>> Peter, do you have any advice from the armv7m side?
>>>
>>> Nothing concrete. I'm not sure that we'd structure the armv7m stuff the way
>>> we have now if we were writing it from scratch, but it's functional enough.
>>> (In particular, if MMIO regions were part of Device rather than SysBusDevice
>>> then I'd be tempted to suggest that CPUs with MMIO-mapped registers should
>>> directly own their MemoryRegions for them. But they aren't, so we can't do
>>> that.)
>>
>> Having thought about this a little more, I believe that these registers are 
>> part of the "cpu package", because they are shared between the 4 cpu cores 
>> within the package.  Thus their current placement attached to LoongArchCPU 
>> -- as well as the current placement of address_space_iocsr -- is incorrect.
> 
> The extioi hardware design is not friend to software developer, local cpu INTC
> is mixed with board INTC with extioi/iocsr. Local cpu INTC registers should 
> be banked,
> address space is space for local cpu review point.
address space of local cpu INTC should be the same from cpu viewpoint.

> 
> how about put address_space_iocsr as board rather than percpu since there is 
> no concept
> of "cpu package".
> 
> regards
> bibo, mao
>>
>>
>> r~
>