diff mbox series

SMMU permission fault on Dom0 when init vpu_decoder

Message ID 20220530152102.GA883104@EPUAKYIW015D (mailing list archive)
State New, archived
Headers show
Series SMMU permission fault on Dom0 when init vpu_decoder | expand

Commit Message

Oleksii Moisieiev May 30, 2022, 3:21 p.m. UTC
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://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.

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.

Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---
 xen/arch/arm/p2m.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Julien Grall May 30, 2022, 3:44 p.m. UTC | #1
(+ 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@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://lore.kernel.org/xen-devel/alpine.DEB.2.10.1902261501020.20689@sstabellini-ThinkPad-X260/
Stefano Stabellini May 31, 2022, 11:13 p.m. UTC | #2
On Mon, 30 May 2022, Julien Grall wrote:
> (+ 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.

Yes, this is very interesting


> [...]
> 
> > ---
> > 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@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.

I think the intent of the check is to recognize that map_regions_p2mt
was called for a normal memory location and, if so, change the p2m type
to p2m_mmio_direct_nc_x.

As a downstream, the patch below is one of the easiest way to have a
self-contained change to fix the problem described above. However, as
upstream this is the wrong location for the check and also maybe the
wrong check.

For dom0, as Julien said, it is easier because we could just have a
check in map_range_to_domain whether the range we are trying to map is a
reserved_memory range (in pseudo-code):

  if ( reserved_memory )
    p2mt = p2m_mmio_direct_nc_x;

I think that would be doable.

For dom0less domUs and for regular xl domUs it is more difficult because
there is no way to say "I want to reassign this reserved-memory range to
a domU". Reserved-memory doesn't have a special API or mapping
operation today. It would be done via a regular xen,reg or iomem mapping
request which doesn't have a cacheability parameter. It is always
non-cacheable. It is not possible to specify any different cacheability
types or NX type or other mapping attributes.


> Stefano
> attempted to do that a few years ago (see [1]). Maybe we should revive it?

I have a couple of patches to add cacheability for dom0less and also
normal guests:

- one patch to introduce xen,reg-cacheable for dom0less domUs
https://github.com/Xilinx/xen/commit/8dbbf64ebf442f4d6e5772b43e8536fa5566ca94

- one patch to add a cacheability parameter to iomem for xl domUs
https://github.com/Xilinx/xen/commit/67bb93dd0fd338aeef624233fc1793c64b6ab0df

I haven't had the time to upstream either of them yet. They would need
some changes to also cover the p2m_mmio_direct_nc_x case.



> > +    {
> > +        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
>
Julien Grall June 1, 2022, 7:50 a.m. UTC | #3
Hi Stefano,

On 01/06/2022 00:13, Stefano Stabellini wrote:
>>> 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@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.
> 
> I think the intent of the check is to recognize that map_regions_p2mt
> was called for a normal memory location and, if so, change the p2m type
> to p2m_mmio_direct_nc_x.

That would have made sense if it was for a domU. But AFAICT the intent 
is to address the problem for dom0.

Technically, GUEST_RAM0_BASE describes the RAM for the guest and not 
dom0. Maybe they are the same on his HW, but without more details I 
can't confirm that. And therefore...

> 
> As a downstream, the patch below is one of the easiest way to have a
> self-contained change to fix the problem described above.
... there are no way for me to tell whether this patch would still be 
fine for a downstream project.

Cheers,

[...]
Peng Fan June 1, 2022, 7:59 a.m. UTC | #4
> 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.c
> odeaurora.org%2Fexternal%2Fimx%2Fimx-xen&amp;data=05%7C01%7Cpeng.f
> an%40nxp.com%7C91e3a953942d414dcc6208da425006e7%7C686ea1d3bc2b
> 4c6fa92cd99c5c301635%7C0%7C0%7C637895208732114203%7CUnknown%
> 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=no%2BV2ubjGmrsm96NP
> ybeeug4a3BXx3oX7xmylzZCU8E%3D&amp;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.

> 
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@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)))
> +    {
> +        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
Oleksii Moisieiev June 1, 2022, 9:04 a.m. UTC | #5
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_moisieiev@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
Oleksii Moisieiev June 1, 2022, 9:13 a.m. UTC | #6
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*2Fsource.c__;JSUl!!GF_29dbcQIUBPA!wzoDdJsuf4bjXMe85tA46E0tLpFG5gqHoo-OeY6o_ARroNBmX7JByHW67qEUik7bNow0STgvAjR4rBkRu2Ux$ [eur01[.]safelinks[.]protection[.]outlook[.]com]
> > odeaurora.org%2Fexternal%2Fimx%2Fimx-xen&amp;data=05%7C01%7Cpeng.f
> > an%40nxp.com%7C91e3a953942d414dcc6208da425006e7%7C686ea1d3bc2b
> > 4c6fa92cd99c5c301635%7C0%7C0%7C637895208732114203%7CUnknown%
> > 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> > CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=no%2BV2ubjGmrsm96NP
> > ybeeug4a3BXx3oX7xmylzZCU8E%3D&amp;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.

> 
> > 
> > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@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)))
> > +    {
> > +        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
Peng Fan June 1, 2022, 9:28 a.m. UTC | #7
> 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fur
> > >
> 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&amp;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&amp;sd
> ata=
> > >
> 47ddyB8JUz8sDHcXluhcB7RJ7bH4a33l%2FF2XzUpAPxY%3D&amp;reserved=0
> > > [eur01[.]safelinks[.]protection[.]outlook[.]com]
> > >
> odeaurora.org%2Fexternal%2Fimx%2Fimx-xen&amp;data=05%7C01%7Cpeng.f
> > >
> an%40nxp.com%7C91e3a953942d414dcc6208da425006e7%7C686ea1d3bc2b
> > >
> 4c6fa92cd99c5c301635%7C0%7C0%7C637895208732114203%7CUnknown%
> > >
> 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> > >
> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=no%2BV2ubjGmrsm96NP
> > > ybeeug4a3BXx3oX7xmylzZCU8E%3D&amp;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.

> 
> >
> > >
> > > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@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)))
> > > +    {
> > > +        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
Oleksii Moisieiev June 1, 2022, 9:31 a.m. UTC | #8
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&amp;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&amp;sd
> > ata=
> > > >
> > 47ddyB8JUz8sDHcXluhcB7RJ7bH4a33l%2FF2XzUpAPxY%3D&amp;reserved=0
> > > > [eur01[.]safelinks[.]protection[.]outlook[.]com]
> > > >
> > odeaurora.org%2Fexternal%2Fimx%2Fimx-xen&amp;data=05%7C01%7Cpeng.f
> > > >
> > an%40nxp.com%7C91e3a953942d414dcc6208da425006e7%7C686ea1d3bc2b
> > > >
> > 4c6fa92cd99c5c301635%7C0%7C0%7C637895208732114203%7CUnknown%
> > > >
> > 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> > > >
> > CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=no%2BV2ubjGmrsm96NP
> > > > ybeeug4a3BXx3oX7xmylzZCU8E%3D&amp;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@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)))
> > > > +    {
> > > > +        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
Peng Fan June 1, 2022, 9:49 a.m. UTC | #9
> Subject: Re: [Xen-devel] SMMU permission fault on Dom0 when init
> vpu_decoder
> 
> (+ 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.c
> odeaurora.org%2Fexternal%2Fimx%2Fimx-xen&amp;data=05%7C01%7Cpeng.f
> an%40nxp.com%7C726bfc45b45747a655bc08da425351d4%7C686ea1d3bc2b
> 4c6fa92cd99c5c301635%7C0%7C0%7C637895222868692087%7CUnknown%
> 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=KcbEjN80VJS5yq4KMp2g
> NjQuVtx95jKHH5T32ZCj3Do%3D&amp;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.
> 
> 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.

It is not device, VPU could execute code, it has firmware. Just like use
SMMU to isolate M4-core, M4 could execute code.

Regards,
Peng.

> 
> [...]
> 
> > ---
> > 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@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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Fxen-devel%2Falpine.DEB.2.10.1902261501020.20689%40sstabellini
> -ThinkPad-X260%2F&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7C726bfc
> 45b45747a655bc08da425351d4%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C637895222868692087%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000
> %7C%7C%7C&amp;sdata=Y%2B%2Fxslm6wc38jM6zZWshRnTM4tx%2BMpyXaI
> r5BLV1R9Q%3D&amp;reserved=0
> 
> --
> Julien Gral
diff mbox series

Patch

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);
 }