Re: [Xen-devel] [PATCH 1/2 v3] iommu/amd: Fix logic for clearing the IOMMU interrupt bits

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.06.13 at 14:18, Tim Deegan <tim@xxxxxxx> wrote:
> At 11:56 +0100 on 10 Jun (1370865376), Jan Beulich wrote:
>> --- 2013-05-13.orig/xen/drivers/passthrough/amd/iommu_guest.c        
>> 2012-02-10 
> 11:25:54.000000000 +0100
>> +++ 2013-05-13/xen/drivers/passthrough/amd/iommu_guest.c     2013-06-10 
> 12:32:00.000000000 +0200
>> @@ -754,7 +754,14 @@ static void guest_iommu_mmio_write64(str
>>          u64_to_reg(&iommu->ppr_log.reg_tail, val);
>>          break;
>>      case IOMMU_STATUS_MMIO_OFFSET:
>> -        u64_to_reg(&iommu->reg_status, val);
>> +        val &= IOMMU_STATUS_EVENT_OVERFLOW_MASK |
>> +               IOMMU_STATUS_EVENT_LOG_INT_MASK |
>> +               IOMMU_STATUS_COMP_WAIT_INT_MASK |
>> +               IOMMU_STATUS_PPR_LOG_OVERFLOW_MASK |
>> +               IOMMU_STATUS_PPR_LOG_INT_MASK |
>> +               IOMMU_STATUS_GAPIC_LOG_OVERFLOW_MASK |
>> +               IOMMU_STATUS_GAPIC_LOG_INT_MASK;
>> +        u64_to_reg(&iommu->reg_status, reg_to_u64(iommu->reg_status) ^ val);
> 
> '^ val'?  Surely '& ~val' or it will set unset bits too.

Oh, yes, of course.

>> @@ -439,8 +439,8 @@ static void iommu_reset_log(struct amd_i
>>  
>>      ctrl_func(iommu, IOMMU_CONTROL_DISABLED);
>>  
>> -    /*clear overflow bit */
>> -    iommu_clear_bit(&entry, of_bit);
>> +    /* RW1C overflow bit */
>> +    iommu_set_bit(&entry, of_bit);
>>      writel(entry, iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
> 
> Won't this clear all other status bits as well?  

Yes it will, albeit not because of this patch. But I guess we should
fix this as we go. Expect v4 soon...

Jan


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