On Mon, May 30, 2022 at 04:44:36PM +0100, Julien Grall wrote: Hi Julien, > (+ Stefano) > > On 30/05/2022 16:21, Oleksii Moisieiev wrote: > > Hello, > > Hi Oleksii, > > > I'm getting permission fault from SMMU when trying to init > > VPU_Encoder/Decoder > > in Dom0 on IMX8QM board: > > (XEN) smmu: /iommu@51400000: Unhandled context fault: fsr=0x408, > > iova=0x86000a60, fsynr=0x1c0062, cb=0 > > This error appears when vpu_encoder/decoder tries to memcpy firmware image > > to > > 0x86000000 address, which is defined in reserved-memory node in xen > > device-tree > > as encoder_boot/decoder_boot region. > > It is not clear to me who is executing the memcpy(). Is it the device or > your domain? If the former, where was the instruction fetch from? > > The reason I am asking that is, from what you wrote, mempcy() will write to > 0x86000000. So the write should not result to a instruction abort. Only an > instruction fetch would lead to such abort.
My configuration is the following: In Dom0 I have vpu_decoder, operated by vpu_malone driver. During initialization, in function vpu_firmware_download it requests firmware and put it to decoder_boot memory using memcpy. Then waiting for the interrupt from the device. Looks like, device decoder tries to execute something from this memory. > > > > > I'm using xen from branch xen-project/staging-4.16 + imx related patches, > > which were > > taken from > > https://urldefense.com/v3/__https://source.codeaurora.org/external/imx/imx-xen__;!!GF_29dbcQIUBPA!xy4tOkXLiMzvC0wg_Me93zTZ4sZBZ7dq_-zkwYSaJvqt5vNVEOa-mV7Li2crSK3OBTQFb396tUDElwtpiw$ > > [source[.]codeaurora[.]org]. > > > > After some investigation I found that this issue was fixed by Peng Fan in > > commit: 46b3dd3718144ca6ac2c12a3b106e57fb7156554 (Hash from codeaurora), > > but only for > > the Guest domains. > > It introduces new p2m_type p2m_mmio_direct_nc_x, which differs from > > p2m_mmio_direct_nc by XN = 0. This type is set to the reserved memory > > region in > > map_mmio_regions function. > > > > I was able to fix issue in Dom0 by setting p2m_mmio_direct_nc_x type for the > > reserved memory in map_regions_p2mt, which is used to map memory during > > Dom0 creation. > > Patch can be found below. > > > > Based on initial discussions on IRC channel - XN bit did the trick because > > looks > > like vpu decoder is executing some code from this memory. > > This was a surprise to me that device could also execute memory. From the > SMMU spec, this looks a legit things. Before relaxing the type, I would like > to confirm this is what's happenning in your case. > > [...] > > > --- > > arm: Set p2m_type to p2m_mmio_direct_nc_x for reserved memory > > regions > > > > This is the enhancement of the 46b3dd3718144ca6ac2c12a3b106e57fb7156554. > > Those patch introduces p2m_mmio_direct_nc_x p2m type which sets the > > e->p2m.xn = 0 for the reserved-memory, such as vpu encoder/decoder. > > > > Set p2m_mmio_direct_nc_x in map_regions_p2mt for reserved-memory the > > same way it does in map_mmio_regions. This change is for the case > > when vpu encoder/decoder works in DomO and not passed-through to the > > Guest Domains. > > > > Signed-off-by: Oleksii Moisieiev <oleksii_moisie...@epam.com> > > --- > > xen/arch/arm/p2m.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > > index e9568dab88..bb1f681b71 100644 > > --- a/xen/arch/arm/p2m.c > > +++ b/xen/arch/arm/p2m.c > > @@ -1333,6 +1333,13 @@ int map_regions_p2mt(struct domain *d, > > mfn_t mfn, > > p2m_type_t p2mt) > > { > > + if (((long)gfn_x(gfn) >= (GUEST_RAM0_BASE >> PAGE_SHIFT)) && > > + (((long)gfn_x(gfn) + nr) <= > > + ((GUEST_RAM0_BASE + GUEST_RAM0_SIZE)>> PAGE_SHIFT))) > > I am afraid I don't understand what this check is for. In a normal setup, we > don't know where the reserved regions are mapped. Only the caller may know > that. > > For dom0, this decision could be taken in map_range_to_domain(). For the > domU, we would need to let the toolstack to chose the memory attribute. > Stefano attempted to do that a few years ago (see [1]). Maybe we should > revive it? > > > + { > > + p2m_remove_mapping(d, gfn, nr, mfn); > > + return p2m_insert_mapping(d, gfn, nr, mfn, p2m_mmio_direct_nc_x); > > + } > > return p2m_insert_mapping(d, gfn, nr, mfn, p2mt); > > } > > Cheers, > > [1] > https://urldefense.com/v3/__https://lore.kernel.org/xen-devel/alpine.DEB.2.10.1902261501020.20689@sstabellini-ThinkPad-X260/__;!!GF_29dbcQIUBPA!xy4tOkXLiMzvC0wg_Me93zTZ4sZBZ7dq_-zkwYSaJvqt5vNVEOa-mV7Li2crSK3OBTQFb396tUBARsu3hw$ > [lore[.]kernel[.]org] > > -- > Julien Gral
