Re: [Xen-devel] [PATCH v8 8/8] xen/arm: map reserved-memory regions as normal memory in dom0

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 7 Nov 2018, Julien Grall wrote:
> On 07/11/2018 12:18, Julien Grall wrote:
> > Hi Stefano,
> > 
> > On 07/11/2018 00:32, Stefano Stabellini wrote:
> > > On Mon, 22 Oct 2018, Julien Grall wrote:
> > > > Hi,
> > > > 
> > > > On 09/10/2018 00:37, Stefano Stabellini wrote:
> > > > > reserved-memory regions should be mapped as normal memory.
> > > > 
> > > > This is already the case with p2m_mmio_direct_c. The hardware domain
> > > > should
> > > > have full control on the resulting attributes via its stage-1 mappings.
> > > > So
> > > > what's wrong with that p2m type?
> > > 
> > > Shared mappings are prevented for any types other than p2m_ram_rw, see
> > > the p2m_is_ram checks in the implementation of XENMAPSPACE_gmfn_share.
> > 
> > This does not make any sense. This series is about mapping between any
> > domain but dom0. So if you end-up to map the reserved-memory region in dom0,
> > why are you using XENMAPSPACE_gmfn_share?
> 
> To clarify my question, what are you trying to achieve? Are you trying to
> share memory between Dom0 and a guest. Or are you trying to share memory
> between an external entity (i.e R* core/device) and the guest?

I have in my TODO list to achieve both the goals you mentioned. However,
with this patch I am trying to enable shared cacheable memory between
Dom0 and a guest. Specifically, I am setting up Dom0 as "owner" (with
the new terminology, formerly "master"), and a DomU as "borrower".

A lot of the steps automated by libxl have to be done manually, such as
advertising the memory region as "reserved-memory" on the Dom0 device
tree and adding the "owner" entries to xenstore, but once that is done,
it works just fine.


> > > The alternative is to remove or extend the p2m_is_ram check at
> > > xen/arch/arm/mm.c:1283
> > 
> > p2m_ram_* means the page is managed by Xen and accounting will be done.
> > Similarly XENMAPSPACE_gmfn_share will do accounting on the page mapped
> > through that.
> > 
> > In the case of reserved-memory, we never handled them properly in Xen (see
> > [1]).
> > 
> > There are 2 types of reserved-memory region: static and dynamic. The dynamic
> > one are not a concerned as addressed are not specified in them.
> > 
> > In the case of static one, they are backed by a page in Xen because we
> > didn't updated the code to carve them out from xenheap. This means that you
> > are mapping those pages in Dom0, yet they may not be assigned to Dom0 and
> > may get allocated for Xen internal use or to another domain.
> > 
> > As such, this patch is just a workaround to an already broken code. So the
> > first step is fixing the brokenness of reserved-memory region. Then we can
> > discuss whether this patch is relevant for any of your use case.

By fixing the brokenness of reserved-memory region, you mean remove them
from xenheap? Anything else you can think of that doesn't work right?


> > Cheers,
> > 
> > [1]
> > https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg02674.html
> > 
> > > 
> > > 
> > > > >   At the
> > > > > moment, they get remapped as device memory because Xen doesn't know
> > > > > better. Add an explicit check for it.
> > > > > 
> > > > > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > > > > ---
> > > > >    xen/arch/arm/domain_build.c | 7 +++++++
> > > > >    1 file changed, 7 insertions(+)
> > > > > 
> > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > > > index f552154..c7df4cf 100644
> > > > > --- a/xen/arch/arm/domain_build.c
> > > > > +++ b/xen/arch/arm/domain_build.c
> > > > > @@ -1307,6 +1307,13 @@ static int __init handle_node(struct domain *d,
> > > > > struct kernel_info *kinfo,
> > > > >                   "WARNING: Path %s is reserved, skip the node as we
> > > > > may
> > > > > re-use the path.\n",
> > > > >                   path);
> > > > >    +    /*
> > > > > +     * reserved-memory ranges should be mapped as normal memory in
> > > > > the
> > > > > +     * p2m.
> > > > > +     */
> > > > > +    if ( !strcmp(dt_node_name(node), "reserved-memory") )
> > > > > +        p2mt = p2m_ram_rw;
> > > > > +
> > > > >        res = handle_device(d, node, p2mt);
> > > > >        if ( res)
> > > > >            return res;
> > > > > 
> > > > 
> > > > -- 
> > > > Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel