Re: [Qemu-devel] [RFC v5 2/8] hw/arm/smmuv3: smmuv3 emulation model

[Auger Eric]

Hi Tomasz,

On 13/07/2017 14:00, Tomasz Nowicki wrote:
> Hi Eric,
> 
> On 09.07.2017 22:51, Eric Auger wrote:
>> From: Prem Mallappa <address@hidden>
>>
>> Introduces the SMMUv3 derived model. This is based on
>> System MMUv3 specification (v17).
>>
>> Signed-off-by: Prem Mallappa <address@hidden>
>> Signed-off-by: Eric Auger <address@hidden>
>>
>> ---
>> v4 -> v5:
>> - change smmuv3_translate proto (IOMMUAccessFlags flag)
>> - has_stagex replaced by is_ste_stagex
>> - smmu_cfg_populate removed
>> - added smmuv3_decode_config and reworked error management
>> - remwork the naming of IOMMU mrs
>> - fix SMMU_CMDQ_CONS offset
>>
> 
> [...]
> 
>> +
>> +/*****************************
>> + *  Register Access Primitives
>> + *****************************/
>> +
>> +static inline void smmu_write64_reg(SMMUV3State *s, uint32_t addr,
>> uint64_t val)
>> +{
>> +    addr >>= 2;
>> +    s->regs[addr] = val & 0xFFFFFFFFULL;
>> +    s->regs[addr + 1] = val & ~0xFFFFFFFFULL;
>> +}
>> +
>> +static inline void smmu_write_reg(SMMUV3State *s, uint32_t addr,
>> uint64_t val)
>> +{
>> +    s->regs[addr >> 2] = val;
>> +}
>> +
>> +static inline uint32_t smmu_read_reg(SMMUV3State *s, uint32_t addr)
>> +{
>> +    return s->regs[addr >> 2];
>> +}
>> +
>> +static inline uint64_t smmu_read64_reg(SMMUV3State *s, uint32_t addr)
>> +{
>> +    addr >>= 2;
>> +    return s->regs[addr] | (s->regs[addr + 1] << 32);
> 
> To be consistent with smmu_write64_reg() we should not shift here second
> half of register, instead simply:
> 
> return s->regs[addr] | s->regs[addr + 1];

Thanks for spotting this. I think regs should be uint32_t instead and
extract64() could be used on write64 and shift would stay on read64().

Regards

Eric
> 
> Thanks,
> Tomasz