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

[Julien Grall <julien@xxxxxxx>]

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@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.
> 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,

[...]

--
Julien Grall