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

RE: [PATCH 07/10] xen/arm: intruduce alloc_domstatic_pages


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Penny Zheng <Penny.Zheng@xxxxxxx>
  • Date: Tue, 18 May 2021 08:57:55 +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-SenderADCheck; bh=JIudVAO9QCkK7W0L6n9pcEes+ogSsARVdoirSSecxg0=; b=VS8tOm6VTh1Sjt6Bdm/NIo3XoS8VIUCVwCPOJvCIA6d+Ghretz+KOb5z2TCtzUrn8U4gQTbA6gvGlGw/qY9JEFctw6H2lWTY6znk1D4esjGwQ1eQa+QbeZROQlNR5cup/A7ArM/3kuPsvhjrDj3VXII81ICmTgVQDrlnv2KEZa6YLwZjzq9X59S1FTxHVuxDzv2J0S9hQ2gr+rwHxX4xguuhMT5Ud2fGC6UJi0etQltqKNwulbmCljs+PzERJEJzKNA5gYYDTNryxTXJZYb8sYk7scaUznR8jLUsxyuOjA/IIijWVW2nd2F74Sr0mRnPNS3YhZxxAlA7OKEsZwN3EQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=b13cvZZqvgU8GELv+TafYpGXW7fHdmVgGJJsWcO+m3UQLTFk708dw1H5/aZo0HZHxbXTFk5pC/nSPtdUQkBFXXr+xcT5mJkYxfWPt+jlWW2pYzEtJprHg7srTnWVBQRjIDIl6N+XQSJhnetR5wdfLw23/K4cKJNAdsd0anHeq3ZD9mQZ+OKS6+PmsGXF1sGud7fuafOmJKfo302UjiYD8zbOls3Tp+twRpH0vOOUA57SXcrAZjDjrWvFtXifXe0jthnDWsleIGPLwG5KxpUbgUzLXM3r5TkZvXTqh55lGhEeVTemVaOFjLhsVfwre3PNInuP5YQWjnDZBtP0/roc0w==
  • Authentication-results-original: suse.com; dkim=none (message not signed) header.d=none;suse.com; dmarc=none action=none header.from=arm.com;
  • Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Wei Chen <Wei.Chen@xxxxxxx>, nd <nd@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, "julien@xxxxxxx" <julien@xxxxxxx>
  • Delivery-date: Tue, 18 May 2021 08:58:16 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: suse.com; dkim=none (message not signed) header.d=none;suse.com; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHXS6W/k/joZEkeaUiDjrfYH3jVEaro2ToAgAAR3sA=
  • Thread-topic: [PATCH 07/10] xen/arm: intruduce alloc_domstatic_pages

Hi Jan

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Tuesday, May 18, 2021 3:35 PM
> To: Penny Zheng <Penny.Zheng@xxxxxxx>
> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; Wei Chen
> <Wei.Chen@xxxxxxx>; nd <nd@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> sstabellini@xxxxxxxxxx; julien@xxxxxxx
> Subject: Re: [PATCH 07/10] xen/arm: intruduce alloc_domstatic_pages
> 
> On 18.05.2021 07:21, Penny Zheng wrote:
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -2447,6 +2447,9 @@ int assign_pages(
> >      {
> >          ASSERT(page_get_owner(&pg[i]) == NULL);
> >          page_set_owner(&pg[i], d);
> > +        /* use page_set_reserved_owner to set its reserved domain owner.
> */
> > +        if ( (pg[i].count_info & PGC_reserved) )
> > +            page_set_reserved_owner(&pg[i], d);
> 
> Now this is puzzling: What's the point of setting two owner fields to the same
> value? I also don't recall you having introduced
> page_set_reserved_owner() for x86, so how is this going to build there?
> 

Thanks for pointing out that it will fail on x86.
As for the same value, sure, I shall change it to domid_t domid to record its 
reserved owner.
Only domid is enough for differentiate. 
And even when domain get rebooted, struct domain may be destroyed, but domid 
will stays
The same.
Major user cases for domain on static allocation are referring to the whole 
system are static,
No runtime creation.

> > @@ -2509,6 +2512,56 @@ struct page_info *alloc_domheap_pages(
> >      return pg;
> >  }
> >
> > +/*
> > + * Allocate nr_pfns contiguous pages, starting at #start, of static
> > +memory,
> > + * then assign them to one specific domain #d.
> > + * It is the equivalent of alloc_domheap_pages for static memory.
> > + */
> > +struct page_info *alloc_domstatic_pages(
> > +        struct domain *d, unsigned long nr_pfns, paddr_t start,
> > +        unsigned int memflags)
> > +{
> > +    struct page_info *pg = NULL;
> > +    unsigned long dma_size;
> > +
> > +    ASSERT(!in_irq());
> > +
> > +    if ( memflags & MEMF_no_owner )
> > +        memflags |= MEMF_no_refcount;
> > +
> > +    if ( !dma_bitsize )
> > +        memflags &= ~MEMF_no_dma;
> > +    else
> > +    {
> > +        dma_size = 1ul << bits_to_zone(dma_bitsize);
> > +        /* Starting address shall meet the DMA limitation. */
> > +        if ( dma_size && start < dma_size )
> > +            return NULL;
> 
> It is the entire range (i.e. in particular the last byte) which needs to meet 
> such
> a restriction. I'm not convinced though that DMA width restrictions and static
> allocation are sensible to coexist.
> 

FWIT, if starting address meets the limitation, the last byte, greater than 
starting
address shall meet it too.

> > +    }
> > +
> > +    pg = alloc_staticmem_pages(nr_pfns, start, memflags);
> > +    if ( !pg )
> > +        return NULL;
> > +
> > +    if ( d && !(memflags & MEMF_no_owner) )
> > +    {
> > +        if ( memflags & MEMF_no_refcount )
> > +        {
> > +            unsigned long i;
> > +
> > +            for ( i = 0; i < nr_pfns; i++ )
> > +                pg[i].count_info = PGC_extra;
> > +        }
> 
> Is this as well as the MEMF_no_owner case actually meaningful for statically
> allocated pages?
> 

Thanks for pointing out. Truly, we do not need to take it considered.

> Jan

 


Rackspace

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