Hi Julien,

> On 3 Dec 2022, at 9:16 pm, Julien Grall <[email protected]> wrote:
> 
> Hi Rahul,
> 
> I have only skimmed through the patch so far.
> 
> On 01/12/2022 16:02, Rahul Singh wrote:
>>  static int vsmmuv3_mmio_write(struct vcpu *v, mmio_info_t *info,
>>                                register_t r, void *priv)
>>  {
>> +    struct virt_smmu *smmu = priv;
>> +    uint64_t reg;
>> +    uint32_t reg32;
>> +
>> +    switch ( info->gpa & 0xffff )
>> +    {
>> +    case VREG32(ARM_SMMU_CR0):
> 
> 
> Shouldn't this code (and all the other register emulations) be protected for 
> concurrent access in some way?

Yes, I agree I will add the lock for register emulations in next v2.
> 
> 
>> +        reg32 = smmu->cr[0];
>> +        vreg_reg32_update(&reg32, r, info);
>> +        smmu->cr[0] = reg32;
>> +        smmu->cr0ack = reg32 & ~CR0_RESERVED;
> 
> Looking at the use. I think it doesn't look necessary to have a copy of cr0 
> with just the reserved bit(s) unset. Instead, it would be better to clear the 
> bit(s) when reading it.

Ack. 
 
Regards,
Rahul

Reply via email to