| Message ID | 20200507143201.31080-4-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | VIRTIO-IOMMU probe request support and MSI bypass on ARM | expand |
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/? For this case (high==low+1) I guess maybe it's also easier to use "addr >= low && addr < high". 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 >
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 >> >
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c index 95059eff70..d35fc9522b 100644 --- a/hw/virtio/virtio-iommu.c +++ b/hw/virtio/virtio-iommu.c @@ -607,6 +607,7 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr, uint32_t sid, flags; bool bypass_allowed; bool found; + int i; interval.low = addr; interval.high = addr + 1; @@ -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) { + 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",
When translating an address we need to check if it belongs to a reserved virtual address range. If it does, there are 2 cases: - it belongs to a RESERVED region: the guest should neither use this address in a MAP not instruct the end-point to DMA on them. We report an error - It belongs to an MSI region: we bypass the translation. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- v11 -> v12: - s/Interval/ReservedRegion - break instead of goto unlock as peter suggested - added VIRTIO_IOMMU_FAULT_F_ADDRESS flag (jean) v10 -> v11: - directly use the reserved_regions properties array v9 -> v10: - in case of MSI region, we immediatly return --- hw/virtio/virtio-iommu.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)