| Message ID | 1456121379-13434-1-git-send-email-aik@ozlabs.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 22, 2016 at 05:09:39PM +1100, Alexey Kardashevskiy wrote: > Since a788f227 "memory: Allow replay of IOMMU mapping notifications" > when new VFIO listener is added, all existing IOMMU mappings are replayed. > However there is a problem that the base address of an IOMMU memory region > (IOMMU MR) is ignored which is not a problem for the existing user (which is > pseries) with its default 32bit DMA window starting at 0 but it is if there is > another DMA window. > > This adjusts the replaying address by mr->addr. Uh.. this doesn't look right to me. AFAICT from the existing implementations the 'addr' parameter to the translate function is an offset within the memory region, which would make the original version correct. > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > memory.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/memory.c b/memory.c > index 09041ed..377269b 100644 > --- a/memory.c > +++ b/memory.c > @@ -1436,7 +1436,7 @@ void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, > IOMMUTLBEntry iotlb; > > for (addr = 0; addr < memory_region_size(mr); addr += granularity) { > - iotlb = mr->iommu_ops->translate(mr, addr, is_write); > + iotlb = mr->iommu_ops->translate(mr, mr->addr + addr, is_write); > if (iotlb.perm != IOMMU_NONE) { > n->notify(n, &iotlb); > }
On 02/22/2016 05:26 PM, David Gibson wrote: > On Mon, Feb 22, 2016 at 05:09:39PM +1100, Alexey Kardashevskiy wrote: >> Since a788f227 "memory: Allow replay of IOMMU mapping notifications" >> when new VFIO listener is added, all existing IOMMU mappings are replayed. >> However there is a problem that the base address of an IOMMU memory region >> (IOMMU MR) is ignored which is not a problem for the existing user (which is >> pseries) with its default 32bit DMA window starting at 0 but it is if there is >> another DMA window. >> >> This adjusts the replaying address by mr->addr. > > Uh.. this doesn't look right to me. AFAICT from the existing > implementations the 'addr' parameter to the translate function is an > offset within the memory region, which would make the original version > correct. Ok, then spapr_tce_translate_iommu() needs to be fixed. Or I am missing something here? The @addr field is definitely ignored now. > >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> memory.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/memory.c b/memory.c >> index 09041ed..377269b 100644 >> --- a/memory.c >> +++ b/memory.c >> @@ -1436,7 +1436,7 @@ void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, >> IOMMUTLBEntry iotlb; >> >> for (addr = 0; addr < memory_region_size(mr); addr += granularity) { >> - iotlb = mr->iommu_ops->translate(mr, addr, is_write); >> + iotlb = mr->iommu_ops->translate(mr, mr->addr + addr, is_write); >> if (iotlb.perm != IOMMU_NONE) { >> n->notify(n, &iotlb); >> } >
On Mon, Feb 22, 2016 at 10:36:19PM +1100, Alexey Kardashevskiy wrote: > On 02/22/2016 05:26 PM, David Gibson wrote: > >On Mon, Feb 22, 2016 at 05:09:39PM +1100, Alexey Kardashevskiy wrote: > >>Since a788f227 "memory: Allow replay of IOMMU mapping notifications" > >>when new VFIO listener is added, all existing IOMMU mappings are replayed. > >>However there is a problem that the base address of an IOMMU memory region > >>(IOMMU MR) is ignored which is not a problem for the existing user (which is > >>pseries) with its default 32bit DMA window starting at 0 but it is if there is > >>another DMA window. > >> > >>This adjusts the replaying address by mr->addr. > > > >Uh.. this doesn't look right to me. AFAICT from the existing > >implementations the 'addr' parameter to the translate function is an > >offset within the memory region, which would make the original version > >correct. > > Ok, then spapr_tce_translate_iommu() needs to be fixed. Or I am missing > something here? The @addr field is definitely ignored now. I think at least one of us is missing something. In the normal AS translation path, IIUC, it will subtract the mr->addr value to give offsets which are passed to translate. It uses those offsets to find the TCE and returns a translated address. Where else the @addr be used? > > > > > >>Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > >>--- > >> memory.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >>diff --git a/memory.c b/memory.c > >>index 09041ed..377269b 100644 > >>--- a/memory.c > >>+++ b/memory.c > >>@@ -1436,7 +1436,7 @@ void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, > >> IOMMUTLBEntry iotlb; > >> > >> for (addr = 0; addr < memory_region_size(mr); addr += granularity) { > >>- iotlb = mr->iommu_ops->translate(mr, addr, is_write); > >>+ iotlb = mr->iommu_ops->translate(mr, mr->addr + addr, is_write); > >> if (iotlb.perm != IOMMU_NONE) { > >> n->notify(n, &iotlb); > >> } > > > >
diff --git a/memory.c b/memory.c index 09041ed..377269b 100644 --- a/memory.c +++ b/memory.c @@ -1436,7 +1436,7 @@ void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, IOMMUTLBEntry iotlb; for (addr = 0; addr < memory_region_size(mr); addr += granularity) { - iotlb = mr->iommu_ops->translate(mr, addr, is_write); + iotlb = mr->iommu_ops->translate(mr, mr->addr + addr, is_write); if (iotlb.perm != IOMMU_NONE) { n->notify(n, &iotlb); }
Since a788f227 "memory: Allow replay of IOMMU mapping notifications" when new VFIO listener is added, all existing IOMMU mappings are replayed. However there is a problem that the base address of an IOMMU memory region (IOMMU MR) is ignored which is not a problem for the existing user (which is pseries) with its default 32bit DMA window starting at 0 but it is if there is another DMA window. This adjusts the replaying address by mr->addr. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)