Re: [PATCH v7 17/28] xen/arm: ITS: Add GITS registers emulation

[Julien Grall <julien.grall@citrix.com>]

On 24/09/15 06:07, Vijay Kilari wrote:
> On Tue, Sep 22, 2015 at 8:00 PM, Julien Grall <julien.grall@citrix.com> wrote:
>> Hi Vijay,
>>
>> On 18/09/15 14:09, vijay.kilari@gmail.com wrote:
> [...]
>>> +    case 0x0010 ... 0x007c:
>>> +    case 0xc000 ... 0xffcc:
>>> +        /* Implementation defined -- write ignored */
>>> +        goto write_ignore;
>>> +    case GITS_CBASER:
>>> +        /* XXX: support 32-bit access */
>>
>> You haven't asked my question on v6... What is missing to support 32-bt
>> support? AFAICT the register helpers should the job for you. Correct?
> 
>    GITS_CBASER.Physical Address holds bits[47:12], Being at
> physical address at bits [47:12]  cannot be updated with single 32-bit access.

I'd like you to explain why GITS_CBASER can't be updated with single
32-bit access. Because, based on the spec it's possible (see 8.19.2 ARM
IHI 0069A):

"Bits [63:32] and bits [31:0] are accessible separately."

> I remember we discussed this on chat.

If you are mentioning the chat we had back in June, this was based on
the hardware spec (PRD03-GENC-009432 23.0) where the support of 32-bit
access wasn't not clearly mandatory.

Although, we had multiple discussion after by the mail with you saying
that 32-bit access should be supported based on the developer spec (IHI
0069A 8.1.3).

After this discussion, you've implemented the 32-bit access on some
register, but left some unintended. A month ago, I provided to you a
patch series which introduce helpers to handle any size access on
register...

While Linux is mostly using 64-bit access on 64-bit register (expect for
GITS_TYPER where there is one 32-bit access), other OS may decide to use
32-bit access because they may want to support GICv3 on aarch32.

We did the mistake to left some access size not implemented on GICv3
which lead to FreeBSD crashing when booting as a Xen guest with GICv3.
We should avoid to reproduce knowingly the same mistake for the ITS.

Regards,

-- 
Julien Grall