2019-05-09 07:11:32

by Heyi Guo

[permalink] [raw]
Subject: Why do we mark vpending table as non-shareable in GICR_VPENDBASER?

Hi Marc,

We can see in its_vpe_schedule() the shareability bits of GICR_VPENDBASER are set as non-shareable, But we set physical PENDBASER as inner-shareable. Is there any special reason for doing this? If it is because the vpending table is GICR specific, why don't we do the same for physical pending table?

We have not seen function issue with this setting, but a special detector in our hardware warns us that there are non-shareable requests sent out while some inner shareable cache entries still present in the cache, and it may cause data inconsistent.

Thanks,

Heyi



2019-05-09 08:00:04

by Marc Zyngier

[permalink] [raw]
Subject: Re: Why do we mark vpending table as non-shareable in GICR_VPENDBASER?

On Thu, 09 May 2019 08:10:09 +0100,
Heyi Guo <[email protected]> wrote:
>
> Hi Marc,
>
> We can see in its_vpe_schedule() the shareability bits of
> GICR_VPENDBASER are set as non-shareable, But we set physical
> PENDBASER as inner-shareable. Is there any special reason for doing
> this? If it is because the vpending table is GICR specific, why
> don't we do the same for physical pending table?

That's a good question. They should have similar attributes.

> We have not seen function issue with this setting, but a special
> detector in our hardware warns us that there are non-shareable
> requests sent out while some inner shareable cache entries still
> present in the cache, and it may cause data inconsistent.

The main issue with the inner-shareable attributes and the GIC is that
nothing in the spec says that CPUs and GIC have to be in the same
inner-shareable domain, as the system can have as many as you want.

You obviously have built it with GICR in the same inner-shareability
domain as the CPU. I'm happy to change the VPENDBASER attributes,
given that the CPU has a mapping to that memory already, and that
shouldn't affect systems where GICR isn't in the same inner shareable
domain anyway.

Thanks,

M.

--
Jazz is not dead, it just smell funny.

2019-05-09 08:58:16

by Heyi Guo

[permalink] [raw]
Subject: Re: Why do we mark vpending table as non-shareable in GICR_VPENDBASER?

Thanks.

One more question about the cacheability of VPROPBASER, which is RaWb, while it is RaWaWb for PROPBASER. Any special reason for this?

Heyi


On 2019/5/9 15:58, Marc Zyngier wrote:
> On Thu, 09 May 2019 08:10:09 +0100,
> Heyi Guo <[email protected]> wrote:
>> Hi Marc,
>>
>> We can see in its_vpe_schedule() the shareability bits of
>> GICR_VPENDBASER are set as non-shareable, But we set physical
>> PENDBASER as inner-shareable. Is there any special reason for doing
>> this? If it is because the vpending table is GICR specific, why
>> don't we do the same for physical pending table?
> That's a good question. They should have similar attributes.
>
>> We have not seen function issue with this setting, but a special
>> detector in our hardware warns us that there are non-shareable
>> requests sent out while some inner shareable cache entries still
>> present in the cache, and it may cause data inconsistent.
> The main issue with the inner-shareable attributes and the GIC is that
> nothing in the spec says that CPUs and GIC have to be in the same
> inner-shareable domain, as the system can have as many as you want.
>
> You obviously have built it with GICR in the same inner-shareability
> domain as the CPU. I'm happy to change the VPENDBASER attributes,
> given that the CPU has a mapping to that memory already, and that
> shouldn't affect systems where GICR isn't in the same inner shareable
> domain anyway.
>
> Thanks,
>
> M.
>


2019-05-09 12:07:44

by Heyi Guo

[permalink] [raw]
Subject: Re: Why do we mark vpending table as non-shareable in GICR_VPENDBASER?

After thinking about it for a while, I feel it difficult to image how non shareable plus normal cacheable works for vpending table. Supposing the shareability bits are to direct the corresponding GICR to read/write table memory, if a vPE is first scheduled on pCPU0 with GICR0 and an VLPI is triggered, so a pending bit in vpending table is set by GICR0 (or ITS?); before the interrupt is activated, the vPE is then scheduled on pCPU1 with GICR1, could GICR1 still be guaranteed to see the pending bit in vpending table when the shareability is non-shareable?

It seems more reasonable for physical pending table to be non-shareable, for it is pinned with one GICR. Even with this assumption, the code will still fall back to non-cacheable cacheability when we have no more choice for shareability attribute other than non-shareable, as the comment says:

/*
* The HW reports non-shareable, we must remove the
* cacheability attributes as well.
*/

Did I miss something?

Thanks,
Heyi

On 2019/5/9 15:58, Marc Zyngier wrote:
> On Thu, 09 May 2019 08:10:09 +0100,
> Heyi Guo <[email protected]> wrote:
>> Hi Marc,
>>
>> We can see in its_vpe_schedule() the shareability bits of
>> GICR_VPENDBASER are set as non-shareable, But we set physical
>> PENDBASER as inner-shareable. Is there any special reason for doing
>> this? If it is because the vpending table is GICR specific, why
>> don't we do the same for physical pending table?
> That's a good question. They should have similar attributes.
>
>> We have not seen function issue with this setting, but a special
>> detector in our hardware warns us that there are non-shareable
>> requests sent out while some inner shareable cache entries still
>> present in the cache, and it may cause data inconsistent.
> The main issue with the inner-shareable attributes and the GIC is that
> nothing in the spec says that CPUs and GIC have to be in the same
> inner-shareable domain, as the system can have as many as you want.
>
> You obviously have built it with GICR in the same inner-shareability
> domain as the CPU. I'm happy to change the VPENDBASER attributes,
> given that the CPU has a mapping to that memory already, and that
> shouldn't affect systems where GICR isn't in the same inner shareable
> domain anyway.
>
> Thanks,
>
> M.
>