qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [Qemu-devel] [kvm-unit-tests PATCH v5 10/11] arm/arm64: g


From: Andre Przywara
Subject: Re: [Qemu-arm] [Qemu-devel] [kvm-unit-tests PATCH v5 10/11] arm/arm64: gicv3: add an IPI test
Date: Fri, 11 Nov 2016 15:20:15 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

Hi,

On 11/11/16 14:53, Alex Bennée wrote:
> 
> Andrew Jones <address@hidden> writes:
> 
>> On Fri, Nov 11, 2016 at 10:02:59AM +0000, Alex Bennée wrote:
>>>
>>> Andrew Jones <address@hidden> writes:
>>>
>>>> On Thu, Nov 10, 2016 at 07:53:58PM +0000, Alex Bennée wrote:
>>>> [...]
>>>>>> +struct gic gicv2 = {
>>>>>> +        .ipi = {
>>>>>> +                .enable = gicv2_enable_defaults,
>>>>>> +                .send_self = gicv2_ipi_send_self,
>>>>>> +                .send_tlist = gicv2_ipi_send_tlist,
>>>>>> +                .send_broadcast = gicv2_ipi_send_broadcast,
>>>>>> +        },
>>>>>> +        .read_iar = gicv2_read_iar,
>>>>>> +        .irqnr = gicv2_irqnr,
>>>>>> +        .write_eoi = gicv2_write_eoi,
>>>>>> +};
>>>>>> +
>>>>>> +struct gic gicv3 = {
>>>>>> +        .ipi = {
>>>>>> +                .enable = gicv3_enable_defaults,
>>>>>> +                .send_self = gicv3_ipi_send_self,
>>>>>> +                .send_tlist = gicv3_ipi_send_tlist,
>>>>>> +                .send_broadcast = gicv3_ipi_send_broadcast,
>>>>>> +        },
>>>>>> +        .read_iar = gicv3_read_iar,
>>>>>> +        .irqnr = gicv3_irqnr,
>>>>>> +        .write_eoi = gicv3_write_eoir,
>>>>>> +};
>>>>>> +
>>>>>
>>>>> So I was re-basing my kvm-unit-tests against your GIC rework and found
>>>>> myself copy and pasting a bunch of this into my tests that fire IRQs.
>>>>> That makes me think the abstraction should be in the library code so
>>>>> other tests can fiddle with sending IRQs.
>>>>>
>>>>> What do you think?
>>>>>
>>>>
>>>> I guess you mean moving the above two structs and their corresponding
>>>> functions (all which aren't already common) to lib/arm/ ? Or do you
>>>> just mean the one non-trivial function gicv3_ipi_send_tlist? I think
>>>> agree with gicv3_ipi_send_tlist getting shared, but the others are
>>>> mostly one-liners, so I'm not sure. I guess I'd have to see how you're
>>>> using them first.
>>>
>>> So it looked like there were some functions in the common code for one
>>> GIC which had local test defined functions for the other. They should at
>>> least be consistent.
>>
>> gicv3_read_iar and gicv3_write_eoir being common already is a product of
>> being sysreg wrappers, allowing for both arm32 and arm64 to use functions
>> of the same names, not because I wanted gicv3 to be inconsistent with
>> gicv2 (which uses MMIO and thus doesn't need wrappers)
>>
>>>
>>> For my use case I could do with a common:
>>>
>>>   gic_enable
>>
>> OK, I can extend gic_init() to initialize a 'struct gic_common_ops' that
>> includes an enable -> *_enable_defaults(void), ipi_send(int cpu),
>> read_iar(void), iar_irqnr(u32 iar), and write_eoi(u32 irqstat). And also
>> provide the wrappers gic_enable, gic_ipi_send(cpu), ...
>>
>>>   gic_send_spi(cpu, irq)
>>
>> I'll let you add this one to the new common ops struct :-)
>>
>>>   gic_irq_ack() which returns the iar.
>>
>> This one will be called read_iar.
>>
>> Would that work for you, Alex?
> 
> Sounds good to me :-)
> 
>>
>> Andre,
>>
>> Would this also satisfy your needs for more common code?

TBH I haven't look deeply enough to give an educated answer. I just
wanted to +1 Alex for the general direction. So I guess it's OK. ;-)

I guess we may need some refactoring later anyway, so any missing pieces
can be added/refactored later once we exactly know what we need.

Cheers,
Andre.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]