Re: [Xen-devel] SMMU permission fault on Dom0 when init vpu_decoder

[Julien Grall <julien@xxxxxxx>]

(+ 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.
> I'm using xen from branch xen-project/staging-4.16 + imx related patches, which 
> were
> taken from https://source.codeaurora.org/external/imx/imx-xen.
>
> 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_moisieiev@xxxxxxxx>
> ---
>   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://lore.kernel.org/xen-devel/alpine.DEB.2.10.1902261501020.20689@sstabellini-ThinkPad-X260/
--
Julien Gral