Re: [PATCH ARM v8 2/4] mini-os: arm: interrupt controller

[Julien Grall <julien.grall@linaro.org>]

On 10/21/2014 04:35 PM, Ian Campbell wrote:
> On Tue, 2014-10-21 at 16:22 +0100, Julien Grall wrote:
>> On 10/21/2014 04:16 PM, Ian Campbell wrote:
>>> On Tue, 2014-10-21 at 15:26 +0100, Julien Grall wrote:
>>>> Our implementation of ITARGETSR doesn't handle correctly read/write per
>>>> byte. If the register is RO (such as for the SGIs and PPIs), our write
>>>> ignore is checking that the guest is writing a word.
>>>
>>> That's a bug.
>>
>> Agree
>>
>>>> Even though we need to fix it in Xen (I could send a patch for it). We
>>>> need to keep this implementation if we want mini-os to run on Xen 4.4.x.
>>>
>>> We should fix bugs, not workaround them in the guest.
>>
>> Writing a 32 bits value into those registers is perfectly value and not
>> a workaround.
> If you would normally write 1 byte (which makes perfect sense for this
> register) but can't because of a Xen bug then avoiding that by
> read/modify/write of 4 bytes because of a Xen bug *is* a workaround.

This case will only happen when the guest is writing in ITARGET for
SPI/PPIs which is RO for PPIs.

The guest may want to write another value than the actual one. Therefore
this could also be considered as a bug from the guest POV.

So we also avoid to avoid writing in ITARGET when it's not necessary.

>> In this case, it would make mini-os working directly on Xen 4.4.

> and how long should mini-os carry this hack for then?

The current issue could be considered as an hardware bug (even though
it's an emulation). Time to time guest has to deal with hardware bug. As
long as it's very simple to fix it and doesn't impact performance, I
guess it's better to have it forever.
In this case, interrupts will be configured only once during mini-os
initialization.

If nobody care about running mini-os on current version of current Xen
4.4.x, then fine.

Regards,

-- 
Julien Grall