Message ID | 1351597670-23031-5-git-send-email-avi@redhat.com |
---|---|
State | New |
Headers | show |
On Tue, Oct 30, 2012 at 11:47 AM, Avi Kivity <avi@redhat.com> wrote: > Accesses which do not translate will hit the fault region, which > can then log the access. Maybe special casing the fault region should be avoided, the IOMMU could set up suitable fault regions by itself. > > Signed-off-by: Avi Kivity <avi@redhat.com> > --- > memory.c | 9 ++++++--- > memory.h | 4 ++++ > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/memory.c b/memory.c > index ba2d4a0..d6b46fd 100644 > --- a/memory.c > +++ b/memory.c > @@ -778,7 +778,9 @@ static void memory_region_destructor_rom_device(MemoryRegion *mr) > static void memory_region_destructor_iommu(MemoryRegion *mr) > { > address_space_destroy(mr->iommu_target_as); > + address_space_destroy(mr->iommu_fault_as); > g_free(mr->iommu_target_as); > + g_free(mr->iommu_fault_as); > } > > static bool memory_region_wrong_endianness(MemoryRegion *mr) > @@ -1001,9 +1003,7 @@ static void memory_region_iommu_rw(MemoryRegion *iommu, hwaddr addr, > xlat = (tlb.translated_addr & ~tlb.addr_mask) | (addr & tlb.addr_mask); > address_space_rw(iommu->iommu_target_as, xlat, buf, clen, is_write); > } else { > - if (!is_write) { > - memset(buf, 0xff, clen); > - } > + address_space_rw(iommu->iommu_fault_as, addr, buf, clen, is_write); > } > buf += clen; > addr += clen; > @@ -1068,6 +1068,7 @@ static void memory_region_iommu_write(void *opaque, hwaddr addr, > void memory_region_init_iommu(MemoryRegion *mr, > MemoryRegionIOMMUOps *ops, > MemoryRegion *target, > + MemoryRegion *fault, > const char *name, > uint64_t size) > { > @@ -1079,6 +1080,8 @@ void memory_region_init_iommu(MemoryRegion *mr, > mr->destructor = memory_region_destructor_iommu; > mr->iommu_target_as = g_new(AddressSpace, 1); > address_space_init(mr->iommu_target_as, target); > + mr->iommu_fault_as = g_new(AddressSpace, 1); > + address_space_init(mr->iommu_fault_as, fault); > } > > static uint64_t invalid_read(void *opaque, hwaddr addr, > diff --git a/memory.h b/memory.h > index 47362c9..6312197 100644 > --- a/memory.h > +++ b/memory.h > @@ -162,6 +162,7 @@ struct MemoryRegion { > unsigned ioeventfd_nb; > MemoryRegionIoeventfd *ioeventfds; > struct AddressSpace *iommu_target_as; > + struct AddressSpace *iommu_fault_as; > }; > > struct MemoryRegionPortio { > @@ -361,12 +362,15 @@ void memory_region_init_reservation(MemoryRegion *mr, > * @ops: a function that translates addresses into the @target region > * @target: a #MemoryRegion that will be used to satisfy accesses to translated > * addresses > + * @fault: a #MemoryRegion for servicing failed translations; accessed with > + * untranslated addresses > * @name: used for debugging; not visible to the user or ABI > * @size: size of the region. > */ > void memory_region_init_iommu(MemoryRegion *mr, > MemoryRegionIOMMUOps *ops, > MemoryRegion *target, > + MemoryRegion *fault, > const char *name, > uint64_t size); > > -- > 1.7.12 >
On 10/30/2012 09:14 PM, Blue Swirl wrote: > On Tue, Oct 30, 2012 at 11:47 AM, Avi Kivity <avi@redhat.com> wrote: >> Accesses which do not translate will hit the fault region, which >> can then log the access. > > Maybe special casing the fault region should be avoided, the IOMMU > could set up suitable fault regions by itself. How can it? Please elaborate. I'm not too pleased with using a MemoryRegion for faults, perhaps a MemoryRegionIOMMUOps::fault() callback would be better.
diff --git a/memory.c b/memory.c index ba2d4a0..d6b46fd 100644 --- a/memory.c +++ b/memory.c @@ -778,7 +778,9 @@ static void memory_region_destructor_rom_device(MemoryRegion *mr) static void memory_region_destructor_iommu(MemoryRegion *mr) { address_space_destroy(mr->iommu_target_as); + address_space_destroy(mr->iommu_fault_as); g_free(mr->iommu_target_as); + g_free(mr->iommu_fault_as); } static bool memory_region_wrong_endianness(MemoryRegion *mr) @@ -1001,9 +1003,7 @@ static void memory_region_iommu_rw(MemoryRegion *iommu, hwaddr addr, xlat = (tlb.translated_addr & ~tlb.addr_mask) | (addr & tlb.addr_mask); address_space_rw(iommu->iommu_target_as, xlat, buf, clen, is_write); } else { - if (!is_write) { - memset(buf, 0xff, clen); - } + address_space_rw(iommu->iommu_fault_as, addr, buf, clen, is_write); } buf += clen; addr += clen; @@ -1068,6 +1068,7 @@ static void memory_region_iommu_write(void *opaque, hwaddr addr, void memory_region_init_iommu(MemoryRegion *mr, MemoryRegionIOMMUOps *ops, MemoryRegion *target, + MemoryRegion *fault, const char *name, uint64_t size) { @@ -1079,6 +1080,8 @@ void memory_region_init_iommu(MemoryRegion *mr, mr->destructor = memory_region_destructor_iommu; mr->iommu_target_as = g_new(AddressSpace, 1); address_space_init(mr->iommu_target_as, target); + mr->iommu_fault_as = g_new(AddressSpace, 1); + address_space_init(mr->iommu_fault_as, fault); } static uint64_t invalid_read(void *opaque, hwaddr addr, diff --git a/memory.h b/memory.h index 47362c9..6312197 100644 --- a/memory.h +++ b/memory.h @@ -162,6 +162,7 @@ struct MemoryRegion { unsigned ioeventfd_nb; MemoryRegionIoeventfd *ioeventfds; struct AddressSpace *iommu_target_as; + struct AddressSpace *iommu_fault_as; }; struct MemoryRegionPortio { @@ -361,12 +362,15 @@ void memory_region_init_reservation(MemoryRegion *mr, * @ops: a function that translates addresses into the @target region * @target: a #MemoryRegion that will be used to satisfy accesses to translated * addresses + * @fault: a #MemoryRegion for servicing failed translations; accessed with + * untranslated addresses * @name: used for debugging; not visible to the user or ABI * @size: size of the region. */ void memory_region_init_iommu(MemoryRegion *mr, MemoryRegionIOMMUOps *ops, MemoryRegion *target, + MemoryRegion *fault, const char *name, uint64_t size);
Accesses which do not translate will hit the fault region, which can then log the access. Signed-off-by: Avi Kivity <avi@redhat.com> --- memory.c | 9 ++++++--- memory.h | 4 ++++ 2 files changed, 10 insertions(+), 3 deletions(-)