[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [PATCH v1 06/13] xen/arm: set up shared memory foreign mapping for borrower domain


  • To: Julien Grall <julien@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Penny Zheng <Penny.Zheng@xxxxxxx>
  • Date: Wed, 20 Apr 2022 08:51:41 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ag9sMdIW+eSpTefTYnIcUxUWvT7sMU2kL25E7VpC+jg=; b=XklvlfJvAgupJblcMzsCSwytOPyLkh4Q8RjzI3iQRO529ybJgEDHEsOGZuN3I9N9m037PKV9OJ3QB0Gqu0EI0pyxBgbss4ljYkwAZWDNVXUmx2lhcs2hqXRx4//VUYSNDAG9xBMkoFNixgs3x0/0WTD60/atDhXX2j97alw95Hc00S2ZW+US8bigL3LTSNIJNfWDdcsDYL8p8UgXugtgE7IswLIuDlclnm20EYwjX1ApzEGZEJwJ5I0OjOeXgQ7MeeHrOyGHgAGN2gxI3yXAMBnDJ2cw3DwWYEYGd6E5KmhWFc3gRJHa1RZAczvKUvx2074bykGQmnQPdEwhb9aHOg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VY/JQC14i9/cedOFIamqvWiFdXJDyvAY80gSUQciC8ThhJ6TnW/Zuj524ZaqEmeyg972ZZLiZw0IRXO35tgRTLHpWGGvAbHWrXMx0hmO04qJLCsFB6YfixpQFmSOUmqRXvnYu85LoFbUOJKseWqbvTpqI48Zobise63D+WzNPxIz380ZxELcRwa3GZceMX7e9eZh7tkaAf2N/aVMzFWd9kJNgOokE3ifC9jLIC3/nACq3OfS6DxXKpguZU/toTesNpXRqzMvfIygInlTRMMDxO/5myKKVwdzpHhG0Jjz3pt8WdK5SmRke0Z88rLW8tm92+TS0oIW2SEs7TLTY9Cevg==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: nd <nd@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Wed, 20 Apr 2022 08:52:02 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHYNQ8JUHwL/QpeQ0iCIDnkwGGL/6zmznEAgBHpDbA=
  • Thread-topic: [PATCH v1 06/13] xen/arm: set up shared memory foreign mapping for borrower domain

Hi julien

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: Saturday, April 9, 2022 6:59 AM
> To: Penny Zheng <Penny.Zheng@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: nd <nd@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Bertrand
> Marquis <Bertrand.Marquis@xxxxxxx>; Volodymyr Babchuk
> <Volodymyr_Babchuk@xxxxxxxx>
> Subject: Re: [PATCH v1 06/13] xen/arm: set up shared memory foreign
> mapping for borrower domain
> 
> Hi Penny,
> 
> On 11/03/2022 06:11, Penny Zheng wrote:
> > From: Penny Zheng <penny.zheng@xxxxxxx>
> >
> > This commits introduces a new helper guest_physmap_add_shm to set up
> > shared memory foreign mapping for borrower domain.
> >
> > Firstly it should get and take reference of statically shared pages
> > from owner dom_shared. Then it will setup P2M foreign memory map of
> > these statically shared pages for borrower domain.
> >
> > This commits only considers owner domain is the default dom_shared,
> > the other scenario will be covered in the following patches.
> >
> > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > ---
> >   xen/arch/arm/domain_build.c | 52
> +++++++++++++++++++++++++++++++++++++
> >   1 file changed, 52 insertions(+)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 984e70e5fc..8cee5ffbd1 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -798,6 +798,48 @@ static int __init allocate_shared_memory(struct
> domain *d,
> >       return ret;
> >   }
> >
> > +static int __init guest_physmap_add_shm(struct domain *od, struct domain
> *bd,
> > +                                        unsigned long o_gfn,
> > +                                        unsigned long b_gfn,
> > +                                        unsigned long nr_gfns) {
> > +    struct page_info **pages = NULL;
> > +    p2m_type_t p2mt, t;
> > +    int ret = 0;
> 
> You don't need to initialize ret.
> 
> > +
> > +    pages = xmalloc_array(struct page_info *, nr_gfns);
> > +    if ( !pages )
> > +        return -ENOMEM;
> > +
> > +    /*
> > +     * Take reference of statically shared pages from owner domain.
> > +     * Reference will be released when destroying shared memory region.
> > +     */
> > +    ret = get_pages_from_gfn(od, o_gfn, nr_gfns, pages, &p2mt, P2M_ALLOC);
> > +    if ( ret )
> > +    {
> > +        ret = -EINVAL;
> > +        goto fail_pages;
> > +    }
> > +
> > +    if ( p2m_is_ram(p2mt) )
> > +        t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw :
> p2m_map_foreign_ro;
> > +    else
> > +    {
> > +        ret = -EINVAL;
> > +        goto fail_pages;
> 
> Where would we release the references?
> 
> > +    }
> > +
> > +    /* Set up guest foreign map. */
> > +    ret = guest_physmap_add_pages(bd, _gfn(b_gfn), page_to_mfn(pages[0]),
> > +                                  nr_gfns, t);
> 
> A few things:
>    - The beginning of the code assumes that the MFN may be discontiguous in
> the physical memory. But here, you are assuming they are contiguous.
> If you want the latter, then you should check the MFNs are contiguous.
> That said, I am not sure if this restriction is really necessary.
> 

Oh,, caught me, it never occurred to me that the pages in array `pages` could
be discontinuous.
I definitely want the latter. This function is always dealing with a memory 
region
(contiguous MFNS) each time. So maybe a loop to check
page_to_mfn(pages[i]) == mfn_add(smfn, i) is needed.

>    - IIRC, guest_physmap_add_pages() doesn't revert the mappings. So you
> need to revert it in case of failure.
> 

Yes, both failure points you are referring to are requiring reverting the 
mappings.

> Cheers,
> 
> --
> Julien Grall

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.