Re: [RFC PATCH 05/24] ARM: GICv3 ITS: introduce ITS command handling

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

Hi Andre,

On 31/01/2017 09:10, Andre Przywara wrote:
> On 02/11/16 15:05, Julien Grall wrote:
>> On 28/09/16 19:24, Andre Przywara wrote:
>>> +}
>>> +
>>> +static uint64_t encode_rdbase(struct host_its *hw_its, int cpu,
>>> uint64_t reg)
>>> +{
>>> +    reg &= ~GENMASK(51, 16);
>>> +
>>> +    if ( hw_its->pta )
>>> +        reg |= per_cpu(rdist_addr, cpu) & GENMASK(51, 16);
>>> +    else
>>> +        reg |= per_cpu(rdist_id, cpu) << 16;
>>
>> I would prefer if we setup the target address at initialize per-cpu
>> rather than doing it every time we send a sync command (or else).
>
> I believe we can't do easily, because the PTA bit is per ITS, not per
> redistributor. I see that it's rather unlikely that we have ITSes with
> different PTA bit settings in one system, but architecturally it's possible.

How about storing the value per ITS then?

[...]

>>> +void gicv3_set_redist_addr(paddr_t address, int redist_id)
>>
>> The second parameter should probably be unsigned, maybe uint64_t?
>
> Well, the processor ID is a 16-bit value and nicely aligns with Xen's
> VCPUIDs, which are declared as "int" in struct vcpu.
> So I'd rather keep it as int here.

I am not sure why you speak about the vCPUIDS where this code only deal 
with pCPUIDs. Anyway, the ID should never be signed and even though it 
has been defined as int in the structure, on ARM we are trying to use 
unsigned number as it makes much more sense.

Furthermore, the per-cpu value rdist_id has been defined as uint64_t, 
hence my request to use uint64_t.

The 2 type need to match for consistency. I am fine if you decide to use 
unsigned int for the per-cpu value.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel