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

RE: [PATCH v2 1/4] docs, xen/arm: Introduce reserved heap memory


  • To: Julien Grall <julien@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Henry Wang <Henry.Wang@xxxxxxx>
  • Date: Tue, 6 Sep 2022 01:09:04 +0000
  • Accept-language: zh-CN, en-US
  • Arc-authentication-results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com])
  • 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=2; 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=Oc/FOGss4EeFcefUfgnZZs9tCthkRP6fMl0fwkk5FRA=; b=KcT7tVchId+A1ggtOrV+cHxTaphSJAICqCJ7V/0NnXXH2IrqKc2l9DkWajj5QVtYnmv2AulnWSi4Kwz44gUh/h3kFxea9WRH0P4lDy6maEIxP1rRgdQ6tp+DECYljglYJVLrSHQ/AZ2l7rCeMR60Lkgl2olZczL0C9ksnvJ28pbQ/NFzI9nNdjvFwEzaa6l38qConIftVYJDcmsXv6E7HVmZGbU/YOuukpeftRGXYzYem3qqeSdiQfIF4ubAWNM+r0Uw2BpuyLR7W3TFS31doROqJTuLDGgE8KEhMnFqks5rGKIYJBnTxa5mi5YZZkDx31F0ieLeJvXx/u/kOKvnZA==
  • 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=Oc/FOGss4EeFcefUfgnZZs9tCthkRP6fMl0fwkk5FRA=; b=XKzWsrE+9TvZvt3l4AM6/9byTe2cdUuw6RFL583xtmV3f7So0QAo4FSOIrTQO9bcmkRI/Dq3PaHnGIaTYvTFQe/RNK8TneChGaV2iezi4yIo0A9awrDR8ydqbzH8xCxYz6beRuX/vVCCz6zpuj1nFsxfLh7RuBG1KR46LmmujS7WblkjNA/hN9i+s2OwjsrsmX5pM92wrEhgKQkjOO6Sm43sT/v/2wWItlJsYmpezTFsCTuFQd+rlW1l+mRsW3BnnurJhBX/qFOGMo95nTZzrTekUlfUGqUXBzcrCWCkk/C+Hi9/dak64AQRMnZK3XDgthRZVdicC45xFdSjSM8ctg==
  • Arc-seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=jroc47vanB76l+PahsXxkZx3Ai2HXJvBHLYTu2ZFI3jfgT2M8p/O/wr/6zvBmMnHZdIkjVQJ5qZ9dRJfpVQYlI6lEhZT0wgHOEY+crd8rc3+1u5Qw5GlpaKkXdWpnYL++RBDAe90tvR3Z2/6+29z/PcP52ARP2hPsTkn95NN0f3IAuSkgSoPl7YNT+TSF14vT4NCpHW6R7Gc5jJRrqn1kDfDsSzuCCHpWXyhwULYc+zhC6J6YAcqiE/Ze2JdfX0XDjmhB/2UXmG8zhsGNM+67ZQZ6TnuOxCHDY81cnW+pvZFePrALizrEpAvR3zKbdfoqMGfNdNsKdBNIwy/36WGMw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kgURaX3z0vbnaLxazQgLnUcYxICzwLdV52YO8rx/MOknFquY5d+u9+hFiEAoKX8qY5+yOs94/hQX22pZn8Z2pwx8FwVHiOBkxyaiARPG+K83EQSJz2PiWJ84p2s6XeJa3bmctEbLXhXtSofbOgVbZBFsYs51aQv29s1GBk3tJaLQp0cETatrpJO2vKdoBRFQBxBxZOtZ8/3CBCBBEmM8KlDvMQpjdbzbBFkQLBZbz31vr6h8Azsx2Ulkq/rwWE/wTBuO1dUVkdXfmOKcEf1J3GEJUkv5UQCvwiufQ1hfH8R9W4iioURn8i4HfLwwxChe3Y39nUtpnx++EyW2VjzW3g==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Wei Chen <Wei.Chen@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Penny Zheng <Penny.Zheng@xxxxxxx>
  • Delivery-date: Tue, 06 Sep 2022 01:09:26 +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: AQHYwPjzoXW29+zrjEaaAKDaLd6E0a3RFcUAgACB/kA=
  • Thread-topic: [PATCH v2 1/4] docs, xen/arm: Introduce reserved heap memory

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Subject: Re: [PATCH v2 1/4] docs, xen/arm: Introduce reserved heap memory
> 
> Hi Henry,
> 
> On 05/09/2022 08:26, Henry Wang wrote:
> > This commit introduces the reserved heap memory, which is parts of RAM
> > reserved in the beginning of the boot time for heap.
> >
> > Firstly, since a new type of memory bank is needed for marking the
> > memory bank solely as the heap, this commit defines `enum
> membank_type`
> 
> The wording is a bit confusing. I read this as the code will use "enum
> membank_type" but this is not possible as your enum is anonymous.
> 
> My suggestion would be to avoid creating a typedef (see below).

Yeah I think you are correct. The typedef is not really necessary.

> 
> > +- For Arm32, since there are seperated heaps, the reserved heap will be
> used
> 
> type: s/seperated/separated/

Oops, sorry again..

> 
> > +typedef enum {
> > +    MEMBANK_MEMORY,
> 
> Technically everything is memory :). I think here you are referring to
> either:
>     - Reserved memory for the device (or firmware)
>     - Any memory that will be used by the allocator.
> 
> I would consider to name the field MEMBANK_UNKNOWN or
> MEMBANK_DEFAULT
> with a comment explaining the meaning depends where it used (we have
> several arrays using struct membank).

MEMBANK_DEFAULT sounds good, and I will add the comment.

> 
> > +    MEMBANK_XEN_DOMAIN, /* whether the memory bank is bound to a
> Xen domain. */
> > +    MEMBANK_RSVD_HEAP, /* whether the memory bank is reserved as
> heap. */
> I would clarify the two values are only valid when the bank in in
> reserved_mem.

Good point, will do.

> 
> > +} membank_type;
> 
> I would prefer if if we don't define any typedef for this enum. But if
> you want to keep it, then please suffix with _t.

No I think you are correct, a enum membank_type instead of a typedef
would be enough here.

Kind regards,
Henry

> 
> >
> >   struct membank {
> >       paddr_t start;
> >       paddr_t size;
> > -    bool xen_domain; /* whether the memory bank is bound to a Xen
> domain. */
> > +    membank_type type;
> >   };
> >
> >   struct meminfo {
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 6e0398f3f6..8d3f859982 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -644,7 +644,7 @@ static void __init init_staticmem_pages(void)
> >
> >       for ( bank = 0 ; bank < bootinfo.reserved_mem.nr_banks; bank++ )
> >       {
> > -        if ( bootinfo.reserved_mem.bank[bank].xen_domain )
> > +        if ( bootinfo.reserved_mem.bank[bank].type ==
> MEMBANK_XEN_DOMAIN )
> >           {
> >               mfn_t bank_start =
> _mfn(PFN_UP(bootinfo.reserved_mem.bank[bank].start));
> >               unsigned long bank_pages =
> PFN_DOWN(bootinfo.reserved_mem.bank[bank].size);
> 
> Cheers,
> 
> --
> Julien Grall

 


Rackspace

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