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

[Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx>]

On Wed, Jun 01, 2022 at 09:28:18AM +0000, Peng Fan wrote:
> > Subject: Re: [Xen-devel] SMMU permission fault on Dom0 when init
> > vpu_decoder
> > 
> > On Wed, Jun 01, 2022 at 07:59:23AM +0000, Peng Fan wrote:
> > > > Subject: [Xen-devel] SMMU permission fault on Dom0 when init
> > > > vpu_decoder
> > > >
> > > > Hello,
> > > >
> > > > 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.
> > > >
> > > > I'm using xen from branch xen-project/staging-4.16 + imx related
> > > > patches, which were taken from
> > > > https://urldefense.com/v3/__https://eur01.safelinks.protection.outlook.com/?url=https*3A*2F*2Fur__;JSUl!!GF_29dbcQIUBPA!xNT11x4E87Ot-pS9c5EbiteNwSWhUuPsM66Y2_ZO5WSMjAMlsRn70_-k8Y2Tfh-GIR018oX6TPa4IEOiIVfv$
> > > >  [eur01[.]safelinks[.]protection[.]outlook[.]com]
> > > >
> > ldefense.com%2Fv3%2F__https%3A%2F%2Feur01.safelinks.protection.outlo
> > > >
> > ok.com%2F%3Furl%3Dhttps*3A*2F*2Fsource.c__%3BJSUl!!GF_29dbcQIUBPA!
> > wz
> > > >
> > oDdJsuf4bjXMe85tA46E0tLpFG5gqHoo-OeY6o_ARroNBmX7JByHW67qEUik7bN
> > ow0ST
> > > >
> > gvAjR4rBkRu2Ux%24&data=05%7C01%7Cpeng.fan%40nxp.com%7C5abe5
> > 7eece
> > > >
> > 404f6c017808da43aef8f7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> > C0%7C
> > > >
> > 637896716019179992%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> > DAiLCJQI
> > > >
> > joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sd
> > ata=
> > > >
> > 47ddyB8JUz8sDHcXluhcB7RJ7bH4a33l%2FF2XzUpAPxY%3D&reserved=0
> > > > [eur01[.]safelinks[.]protection[.]outlook[.]com]
> > > >
> > odeaurora.org%2Fexternal%2Fimx%2Fimx-xen&data=05%7C01%7Cpeng.f
> > > >
> > an%40nxp.com%7C91e3a953942d414dcc6208da425006e7%7C686ea1d3bc2b
> > > >
> > 4c6fa92cd99c5c301635%7C0%7C0%7C637895208732114203%7CUnknown%
> > > >
> > 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> > > >
> > CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=no%2BV2ubjGmrsm96NP
> > > > ybeeug4a3BXx3oX7xmylzZCU8E%3D&reserved=0.
> > > >
> > > > 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.
> > > >
> > > > The purpose of this email is to discuss this issue and probably
> > > > produce generic solution for it.
> > > >
> > > > Best regards,
> > > > Oleksii.
> > > >
> > > > ---
> > > > 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.
> > >
> > > For Dom0, there is no SMMU, so no need x. Just nc is enough.
> > >
> > > Regards,
> > > Peng.
> > 
> > I would say that SMMU is not neccessary for Dom0 because it's mapped 1:1.
> > But using device under SMMU in Dom0 is still valid case. For example to 
> > protect
> > device from reaching address, assigned to another domain, since Dom0 is
> > trusted.
> 
> I mean the reason to use nc_x is that VPU DomU is accessing DRAM through SMMU.
> It needs X because there is VPU firmware run in DomU.
> 
> I not see why need X for Dom0, unless you assign a SID for VPU and create SMMU
> mapping for VPU in Dom0.
> 
> Regards,
> Peng.

That is my case. I've created SMMU mapping for VPU in Dom0 and use
vpu_encoder/decoder from Dom0.

> 
> > 
> > >
> > > >
> > > > 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)))
> > > > +    {
> > > > +        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);  }
> > > >
> > > > --
> > > > 2.27.0