Re: [PATCH 3/5] virtio-iommu: Handle reserved regions in the translation process

[Auger Eric <eric.auger@redhat.com>]

Hi Peter,

On 5/7/20 9:47 PM, Peter Xu wrote:
> Hi, Eric,
> 
> On Thu, May 07, 2020 at 04:31:59PM +0200, Eric Auger wrote:
>> @@ -640,6 +641,24 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>>          goto unlock;
>>      }
>>  
>> +    for (i = 0; i < s->nb_reserved_regions; i++) {
>> +        if (interval.low >= s->reserved_regions[i].low &&
>> +            interval.low <= s->reserved_regions[i].high) {
> 
> Should this be s/low/high/?
I meant addr >= s->reserved_regions[i].low && addr <=
s->reserved_regions[i].high

We only compare a single iova against reserved range boundaries and
boundaries are inclusive
> 
> For this case (high==low+1) I guess maybe it's also easier to use "addr >= low
> && addr < high".
Yes using addr directly is definitively more readable ;-)

Thanks

Eric
> 
> Thanks,
> 
>> +            switch (s->reserved_regions[i].type) {
>> +            case VIRTIO_IOMMU_RESV_MEM_T_MSI:
>> +                entry.perm = flag;
>> +                break;
>> +            case VIRTIO_IOMMU_RESV_MEM_T_RESERVED:
>> +            default:
>> +                virtio_iommu_report_fault(s, VIRTIO_IOMMU_FAULT_R_MAPPING,
>> +                                          VIRTIO_IOMMU_FAULT_F_ADDRESS,
>> +                                          sid, addr);
>> +                break;
>> +            }
>> +            goto unlock;
>> +        }
>> +    }
>> +
>>      if (!ep->domain) {
>>          if (!bypass_allowed) {
>>              error_report_once("%s %02x:%02x.%01x not attached to any domain",
>> -- 
>> 2.20.1
>>
>