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

Re: [Minios-devel] [UNIKRAFT PATCH] plat/*: Replace memregion hardcoded values

To: Florian Schmidt <Florian.Schmidt@xxxxxxxxx>, Cristian Banu <cristb@xxxxxxxxx>, <minios-devel@xxxxxxxxxxxxx>
From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
Date: Tue, 11 Jun 2019 15:05:30 +0200
Cc: costin.lupu@xxxxxxxxx
Delivery-date: Tue, 11 Jun 2019 13:05:43 +0000
List-id: Mini-os development list <minios-devel.lists.xenproject.org>

Hey Christian,

could you revisit this patch under consideration that we have a morecomplicated interface since the introduction of initrd's on KVM?I am not sure yet, how you would build this switch case with enums inour new situation.


Thanks,

Simon

On 12.04.19 10:27, Florian Schmidt wrote:

Hi Christian,

without checking the details of the code, the idea looks fine to me.

Acked-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>

However, Simon has been working on this code recently and run into someadditional complicated cases. It sounds like there are some issues, andwe might actually need a different approach that hopefully might alsonot require having different enums for different platforms either.


Maybe Simon can say something about that.

Cheers,
Florian


On 4/5/19 10:54 AM, Cristian Banu wrote:

This patch replaces the hardcoded values for the memory region indexes
with enums, in order to make the process of adding or removing memory
regions less fragile.

Signed-off-by: Cristian Banu <cristb@xxxxxxxxx>
---
  plat/kvm/memory.c | 33 +++++++++++++++++++++++----------
  plat/xen/memory.c | 30 +++++++++++++++++++++---------
  2 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/plat/kvm/memory.c b/plat/kvm/memory.c
index e45b88c59495..0b6a3cee539d 100644
--- a/plat/kvm/memory.c
+++ b/plat/kvm/memory.c
@@ -31,9 +31,22 @@ extern void *_libkvmplat_heap_start;
  extern void *_libkvmplat_stack_top;
  extern void *_libkvmplat_mem_end;
+enum ukplat_memregions {
+    UKPLAT_MEMREGION_TEXT,
+    UKPLAT_MEMREGION_EH_FRAME,
+    UKPLAT_MEMREGION_EH_FRAME_HDR,
+    UKPLAT_MEMREGION_RODATA,
+    UKPLAT_MEMREGION_CTORS,
+    UKPLAT_MEMREGION_DATA,
+    UKPLAT_MEMREGION_BSS,
+    UKPLAT_MEMREGION_HEAP,
+    UKPLAT_MEMREGION_STACK,
+    UKPLAT_MEMREGION_COUNT
+};
+
  int ukplat_memregion_count(void)
  {
-    return 9;
+    return UKPLAT_MEMREGION_COUNT;
  }
  int ukplat_memregion_get(int i, struct ukplat_memregion_desc *m)

@@ -43,7 +56,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

      UK_ASSERT(m);
      switch (i) {
-    case 0: /* text */
+    case UKPLAT_MEMREGION_TEXT:
          m->base  = (void *) __TEXT;
          m->len   = (size_t) __ETEXT - (size_t) __TEXT;
          m->flags = (UKPLAT_MEMRF_RESERVED

@@ -53,7 +66,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

  #endif
          ret = 0;
          break;
-    case 1: /* eh_frame */
+    case UKPLAT_MEMREGION_EH_FRAME:
          m->base  = (void *) __EH_FRAME_START;
          m->len   = (size_t) __EH_FRAME_END
                  - (size_t) __EH_FRAME_START;

@@ -64,7 +77,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

          m->name  = "eh_frame";
  #endif
          break;
-    case 2: /* eh_frame_hdr */
+    case UKPLAT_MEMREGION_EH_FRAME_HDR:
          m->base  = (void *) __EH_FRAME_HDR_START;
          m->len   = (size_t) __EH_FRAME_HDR_END
                 - (size_t) __EH_FRAME_HDR_START;

@@ -75,7 +88,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

          m->name  = "eh_frame_hdr";
  #endif
          break;
-    case 3: /* rodata */
+    case UKPLAT_MEMREGION_RODATA:
          m->base  = (void *) __RODATA;
          m->len   = (size_t) __ERODATA - (size_t) __RODATA;
          m->flags = (UKPLAT_MEMRF_RESERVED

@@ -85,7 +98,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

  #endif
          ret = 0;
          break;
-    case 4: /* ctors */
+    case UKPLAT_MEMREGION_CTORS:
          m->base  = (void *) __CTORS;
          m->len   = (size_t) __ECTORS - (size_t) __CTORS;
          m->flags = (UKPLAT_MEMRF_RESERVED

@@ -95,7 +108,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

  #endif
          ret = 0;
          break;
-    case 5: /* data */
+    case UKPLAT_MEMREGION_DATA:
          m->base  = (void *) __DATA;
          m->len   = (size_t) __EDATA - (size_t) __DATA;
          m->flags = (UKPLAT_MEMRF_RESERVED

@@ -106,7 +119,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

  #endif
          ret = 0;
          break;
-    case 6: /* bss */
+    case UKPLAT_MEMREGION_BSS:
          m->base  = (void *) __BSS_START;
          m->len   = (size_t) __END - (size_t) __BSS_START;
          m->flags = (UKPLAT_MEMRF_RESERVED

@@ -117,7 +130,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

  #endif
          ret = 0;
          break;
-    case 7: /* heap */
+    case UKPLAT_MEMREGION_HEAP:
          m->base  = _libkvmplat_heap_start;
          m->len   = (size_t) _libkvmplat_stack_top
                 - (size_t) _libkvmplat_heap_start;

@@ -127,7 +140,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

  #endif
          ret = 0;
          break;
-    case 8: /* stack */
+    case UKPLAT_MEMREGION_STACK:
          m->base  = _libkvmplat_stack_top;
          m->len   = (size_t) _libkvmplat_mem_end
                 - (size_t) _libkvmplat_stack_top;
diff --git a/plat/xen/memory.c b/plat/xen/memory.c
index cb1c3552a0d9..d477544b29df 100644
--- a/plat/xen/memory.c
+++ b/plat/xen/memory.c
@@ -45,9 +45,20 @@
  #include <uk/assert.h>
+enum ukplat_memregions {
+    UKPLAT_MEMREGION_TEXT,
+    UKPLAT_MEMREGION_EH_FRAME,
+    UKPLAT_MEMREGION_EH_FRAME_HDR,
+    UKPLAT_MEMREGION_RODATA,
+    UKPLAT_MEMREGION_CTORS,
+    UKPLAT_MEMREGION_DATA,
+    UKPLAT_MEMREGION_BSS,
+    UKPLAT_MEMREGION_COUNT
+};
+
  int ukplat_memregion_count(void)
  {
-    return (int) _libxenplat_mrd_num + 7;
+    return (int) _libxenplat_mrd_num + UKPLAT_MEMREGION_COUNT;
  }
  int ukplat_memregion_get(int i, struct ukplat_memregion_desc *m)

@@ -56,7 +67,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

      UK_ASSERT(m);
      switch (i) {
-    case 0: /* text */
+    case UKPLAT_MEMREGION_TEXT:
          m->base  = (void *) __TEXT;
          m->len   = (size_t) __ETEXT - (size_t) __TEXT;
          m->flags = (UKPLAT_MEMRF_RESERVED

@@ -65,7 +76,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

          m->name  = "text";
  #endif
          break;
-    case 1: /* eh_frame */
+    case UKPLAT_MEMREGION_EH_FRAME:
          m->base  = (void *) __EH_FRAME_START;
          m->len   = (size_t) __EH_FRAME_END
                  - (size_t) __EH_FRAME_START;

@@ -75,7 +86,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

          m->name  = "eh_frame";
  #endif
          break;
-    case 2: /* eh_frame_hdr */
+    case UKPLAT_MEMREGION_EH_FRAME_HDR:
          m->base  = (void *) __EH_FRAME_HDR_START;
          m->len   = (size_t) __EH_FRAME_HDR_END
                  - (size_t) __EH_FRAME_HDR_START;

@@ -85,7 +96,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

          m->name  = "eh_frame_hdr";
  #endif
          break;
-    case 3:    /* ro data */
+    case UKPLAT_MEMREGION_RODATA:
          m->base  = (void *) __RODATA;
          m->len   = (size_t) __ERODATA - (size_t) __RODATA;
          m->flags = (UKPLAT_MEMRF_RESERVED

@@ -94,7 +105,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

          m->name  = "rodata";
  #endif
          break;
-    case 4: /* ctors */
+    case UKPLAT_MEMREGION_CTORS:
          m->base  = (void *) __CTORS;
          m->len   = (size_t) __ECTORS - (size_t) __CTORS;
          m->flags = (UKPLAT_MEMRF_RESERVED

@@ -103,7 +114,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

          m->name  = "ctors";
  #endif
          break;
-    case 5: /* data */
+    case UKPLAT_MEMREGION_DATA:
          m->base  = (void *) __DATA;
          m->len   = (size_t) __EDATA - (size_t) __DATA;
          m->flags = (UKPLAT_MEMRF_RESERVED

@@ -113,7 +124,7 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

          m->name  = "data";
  #endif
          break;
-    case 6: /* bss */
+    case UKPLAT_MEMREGION_BSS:
          m->base  = (void *) __BSS_START;
          m->len   = (size_t) __END - (size_t) __BSS_START;
          m->flags = (UKPLAT_MEMRF_RESERVED

@@ -133,7 +144,8 @@ int ukplat_memregion_get(int i, structukplat_memregion_desc *m)

  #endif
              return -1;
          } else {
-            memcpy(m, &_libxenplat_mrd[i - 7], sizeof(*m));
+            memcpy(m, &_libxenplat_mrd[i - UKPLAT_MEMREGION_COUNT],
+                sizeof(*m));
          }
          break;
      }


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

Prev by Date: Re: [Minios-devel] [UNIKRAFT PATCH v2] lib/ukboot: Fix handling of main arguments
Next by Date: [Minios-devel] [UNIKRAFT PATCH] plat/kvm: Fix current thread retrieval in interrupt context on x86_64
Previous by thread: Re: [Minios-devel] [UNIKRAFT/LWIP PATCH 3/3] Export additional symbols
Next by thread: [Minios-devel] [UNIKRAFT PATCH] plat/kvm: Fix current thread retrieval in interrupt context on x86_64
Index(es):
- Date
- Thread

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