Re: [PATCH][v2] iommu: arm-smmu-v3: Copy SMMU table for kdump kernel

[Will Deacon <will@kernel.org>]

On Tue, Jun 02, 2020 at 07:34:47PM +0530, Prabhakar Kushwaha wrote:
> On Mon, Jun 1, 2020 at 1:10 PM Will Deacon <will@kernel.org> wrote:
> > On Thu, May 21, 2020 at 04:52:02PM +0530, Prabhakar Kushwaha wrote:
> > > On Thu, May 21, 2020 at 2:53 PM Will Deacon <will@kernel.org> wrote:
> > > > On Tue, May 19, 2020 at 08:24:21AM +0530, Prabhakar Kushwaha wrote:
> > > > > What kind of issue you are foreseeing in using memcpy(). May be we can
> > > > > try to find a solution.
> > > >
> > > > Well the thing might not be cache-coherent to start with...
> > > >
> > >
> > > Thanks for telling possible issue area.  Let me try to explain why
> > > this should not be an issue.
> > >
> > > kdump kernel runs from reserved memory space defined during the boot
> > > of first kernel. kdump does not touch memory of the previous kernel.
> > > So no page has been created in kdump kernel  and  there should not be
> > > any data/attribute/coherency issue from MMU point of view .
> >
> > Then how does this work?:
> >
> >         rdcfg.strtab = memremap(rdcfg.strtab_dma, size, MEMREMAP_WB);
> >
> > You're explicitly asking for a write-back mapping.
> >
> 
> As i mentioned earlier, I will replace it with MEMREMAP_WT to make
> sure data is written into the memory.
> 
> Please note, this memmap is temporary for copying older SMMU table to
> cfg->strtab.
> Here, cfg->strtab & cfg->strtab_dma allocated via dmam_alloc_coherent
> during SMMU probe.
> 
> 
> > > During SMMU probe functions,  dmem_alloc_coherent() will be used
> > > allocate new memory (part of existing flow).
> > > This patch copy STE or first level descriptor to *this* memory, after
> > > mapping physical address using memremap().
> > > It just copy everything  so there should not be any issue related to
> > > attribute/content.
> > >
> > > Yes, copying  done after mapping it as MEMREMAP_WB. if you want I can
> > > use it as MEMREMAP_WT
> >
> > You need to take into account whether or not the device is coherent, and the
> > DMA API is designed to handle that for you. But even then, this is fragile
> > as hell because you end up having to infer the hardware configuration
> > from the device to understand the size and format of the data structures.
> > If the crashkernel isn't identical to the host kernel (in terms of kconfig,
> > driver version, firmware tables, cmdline etc) then this is very likely to
> > go wrong.
> 
> There are two possible scenarios for mismatched kdump kernel
> 1.  kdump kernel does not have the devices' driver
> 2.  kdump kernel have the different variation/configuration of driver
> 
> This patch create temporary SMMU table entries which are overwritten
> by driver-probe.

What exactly does this achieve, given that you don't copy the context
descriptors or the page tables?

> Driver's probe will overwrite SMMU entries based on its new
> requirement (size, format, data structures etc).
> 
> for "1",  As  no device driver,  SMMU entry will remain there.
> Means no-one looking for the copied content (even if device continued
> to perform DMA).
> 
> About coherency between Cores and Memory(DMA).
> At the time of crash:  Only one CPU is allowed to remain continue,
> rest are stopped.
> __crash_kexec --> machine_crash_shutdown --> crash_smp_send_stop()
> 
> The active CPU is used to boot kdump kernel. hence none of the CPUs is
> looking for data copied by DMA.
> Coherency issue should not be there.

I'm talking about coherency between the SMMU and the CPU, so I don't think
the number of CPUs is relevant.

> please let me know your view.

It still seems extremely fragile to me, so I continue to think that this
is the wrong approach.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel