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

Re: [Minios-devel] [UNIKRAFT PATCH 4/9] plat/*: Replace uk_printk() with uk_pr_*() equivalents



Hello Simon,

I agree with your proposal to remove the printf with the ukplat_coutk. Please revise it in the next version.

There are couple of minor comments on this patch.

On 09/25/2018 02:52 PM, Simon Kuenzer wrote:
Replace all occurrences of uk_printk() with reasonable uk_pr_*()
equivalents. An exception is in LinuxU platform. The platform's
command line interaction (e.g., `-h`, `-V`) is going to use standard
output.

Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
---
  plat/common/x86/trace.c | 25 ++++++++++++-------------
  plat/common/x86/traps.c |  2 +-
  plat/kvm/shutdown.c     |  2 +-
  plat/linuxu/setup.c     | 24 +++++++++++++-----------
  plat/xen/events.c       | 24 +++++++++++++-----------
  5 files changed, 40 insertions(+), 37 deletions(-)

diff --git a/plat/common/x86/trace.c b/plat/common/x86/trace.c
index 9bb9eeb..dacf1ef 100644
--- a/plat/common/x86/trace.c
+++ b/plat/common/x86/trace.c
@@ -40,19 +40,18 @@
void dump_regs(struct __regs *regs)
  {
-       /* TODO uk_printd() instead of uk_printk() */
-       uk_printk("RIP: %016lx CS: %04lx\n", regs->rip, regs->cs & 0xffff);
-       uk_printk("RSP: %016lx SS: %04lx EFLAGS: %08lx\n",
+       uk_pr_info("RIP: %016lx CS: %04lx\n", regs->rip, regs->cs & 0xffff);
+       uk_pr_info("RSP: %016lx SS: %04lx EFLAGS: %08lx\n",
                        regs->rsp, regs->ss, regs->eflags);
-       uk_printk("RAX: %016lx RBX: %016lx RCX: %016lx\n",
+       uk_pr_info("RAX: %016lx RBX: %016lx RCX: %016lx\n",
                        regs->rax, regs->rbx, regs->rcx);
-       uk_printk("RDX: %016lx RSI: %016lx RDI: %016lx\n",
+       uk_pr_info("RDX: %016lx RSI: %016lx RDI: %016lx\n",
                        regs->rdx, regs->rsi, regs->rdi);
-       uk_printk("RBP: %016lx R08: %016lx R09: %016lx\n",
+       uk_pr_info("RBP: %016lx R08: %016lx R09: %016lx\n",
                        regs->rbp, regs->r8, regs->r9);
-       uk_printk("R10: %016lx R11: %016lx R12: %016lx\n",
+       uk_pr_info("R10: %016lx R11: %016lx R12: %016lx\n",
                        regs->r10, regs->r11, regs->r12);
-       uk_printk("R13: %016lx R14: %016lx R15: %016lx\n",
+       uk_pr_info("R13: %016lx R14: %016lx R15: %016lx\n",
                        regs->r13, regs->r14, regs->r15);
  }
@@ -66,10 +65,10 @@ void dump_mem(unsigned long addr) for (i = ((addr) - 16) & ~15; i < (((addr) + 48) & ~15); i++) {
                if (!(i % 16))
-                       uk_printk("\n%lx:", i);
-               uk_printk(" %02x", *(unsigned char *) i);
+                       uk_pr_info("\n%lx:", i);
+               uk_pr_info(" %02x", *(unsigned char *) i);
        }
-       uk_printk("\n");
+       uk_pr_info("\n");
  }
void stack_walk(void)
@@ -85,8 +84,8 @@ void stack_walk_for_frame(unsigned long frame_base)
  {
        unsigned long *frame = (void *) frame_base;
- uk_printk("base is %#lx ", frame_base);
-       uk_printk("caller is %#lx\n", frame[1]);
+       uk_pr_info("base is %#lx ", frame_base);
+       uk_pr_info("caller is %#lx\n", frame[1]);
        if (frame[0])
                stack_walk_for_frame(frame[0]);
  }
diff --git a/plat/common/x86/traps.c b/plat/common/x86/traps.c
index 434577f..deeb729 100644
--- a/plat/common/x86/traps.c
+++ b/plat/common/x86/traps.c
@@ -63,7 +63,7 @@ void do_unhandled_trap(int trapnr, char *str, struct __regs 
*regs,
  {
        uk_printd(DLVL_CRIT, "Unhandled Trap %d (%s), error code=0x%lx\n",
                        trapnr, str, error_code);
-       uk_printk("Regs address %p\n", regs);
+       uk_pr_info("Regs address %p\n", regs);
        /* TODO revisit when UK_CRASH will also dump the registers */
        dump_regs(regs);
        UK_CRASH("Crashing\n");
diff --git a/plat/kvm/shutdown.c b/plat/kvm/shutdown.c
index 474bfb4..4d8def0 100644
--- a/plat/kvm/shutdown.c
+++ b/plat/kvm/shutdown.c
@@ -32,7 +32,7 @@ static void cpu_halt(void) __noreturn;
  /* TODO: implement CPU reset */
  void ukplat_terminate(enum ukplat_gstate request __unused)
  {
-       uk_printk("Unikraft halted\n");
+       uk_pr_info("Unikraft halted\n");
/* Try to make system off */
        system_off();
diff --git a/plat/linuxu/setup.c b/plat/linuxu/setup.c
index acb67d2..310e666 100644
--- a/plat/linuxu/setup.c
+++ b/plat/linuxu/setup.c
@@ -34,6 +34,7 @@
   */
#include <uk/config.h>
+#include <stdio.h>
  #include <string.h>
  #include <errno.h>
  #include <getopt.h>
@@ -57,20 +58,21 @@ static struct option lopts[] = {
static void version(void)
  {
-       uk_printk("Unikraft "
-                 STRINGIFY(UK_CODENAME) " "
-                 STRINGIFY(UK_FULLVERSION) "\n");
+       printf("Unikraft "
+              STRINGIFY(UK_CODENAME) " "
+              STRINGIFY(UK_FULLVERSION) "\n");
  }
static void usage(const char *progname)
  {
-       uk_printk("Usage: %s [[LINUXU PLATFORM ARGUMENT]].. -- 
[[ARGUMENT]]..\n", progname);
-       uk_printk("\n");
-       uk_printk("Unikraft LinuxU platform arguments:\n");
-       uk_printk("Mandatory arguments to long options are mandatory for short 
options too.\n");
-       uk_printk("  -h, --help                 display this help and exit\n");
-       uk_printk("  -V, --version              display Unikraft version and 
exit\n");
-       uk_printk("  -m, --heapmem [MBYTES]     allocate MBYTES as heap 
memory\n");
+       printf("Usage: %s [[LINUXU PLATFORM ARGUMENT]].. -- [[ARGUMENT]]..\n",
+              progname);
+       printf("\n");
+       printf("Unikraft LinuxU platform arguments:\n");
+       printf("Mandatory arguments to long options are mandatory for short options 
too.\n");
+       printf("  -h, --help                 display this help and exit\n");
+       printf("  -V, --version              display Unikraft version and 
exit\n");
+       printf("  -m, --heapmem [MBYTES]     allocate MBYTES as heap memory\n");
  }
static int parseopts(int argc, char *argv[], struct liblinuxuplat_opts *opts)
@@ -114,7 +116,7 @@ static int parseopts(int argc, char *argv[], struct 
liblinuxuplat_opts *opts)
                                                        * 1024 * 1024);
                        break;
                default:
-                       uk_printk("%s: invalid option: -%c\n", progname, opt);
+                       printf("%s: invalid option: -%c\n", progname, opt);
                        usage(progname);
                        ret = -EINVAL;
                        goto out;
diff --git a/plat/xen/events.c b/plat/xen/events.c
index 1709b90..dad6f9c 100644
--- a/plat/xen/events.c
+++ b/plat/xen/events.c
@@ -70,7 +70,7 @@ void unbind_all_ports(void)
  #endif
if (ukarch_test_and_clr_bit(i, bound_ports)) {
-                       uk_printk("port %d still bound!\n", i);
+                       uk_pr_warn("Port %d still bound!\n", i);
                        unbind_evtchn(i);
                }
        }
@@ -88,7 +88,7 @@ int do_event(evtchn_port_t port, struct __regs *regs)
        clear_evtchn(port);
if (port >= NR_EVS) {
-               uk_printk("WARN: %s: Port number too large: %d\n", __func__, 
port);
+               uk_pr_err("%s: Port number too large: %d\n", __func__, port);
                return 1;
        }
@@ -106,8 +106,8 @@ evtchn_port_t bind_evtchn(evtchn_port_t port, evtchn_handler_t handler,
                          void *data)
  {
        if (ev_actions[port].handler != default_handler)
-               uk_printk("WARN: Handler for port %d already registered, 
replacing\n",
-                               port);
+               uk_pr_warn("Handler for port %d already registered, 
replacing\n",
+                          port);
ev_actions[port].data = data;
        wmb();
@@ -123,7 +123,7 @@ void unbind_evtchn(evtchn_port_t port)
        int rc;
if (ev_actions[port].handler == default_handler)
-               uk_printk("WARN: No handler for port %d when unbinding\n", 
port);
+               uk_pr_warn("No handler for port %d when unbinding\n", port);
        mask_evtchn(port);
        clear_evtchn(port);
@@ -135,7 +135,7 @@ void unbind_evtchn(evtchn_port_t port)
        close.port = port;
        rc = HYPERVISOR_event_channel_op(EVTCHNOP_close, &close);
        if (rc)
-               uk_printk("WARN: close_port %u failed rc=%d. ignored\n", port, 
rc);
+               uk_pr_warn("close_port %u failed rc=%d. ignored\n", port, rc);
} @@ -150,7 +150,8 @@ evtchn_port_t bind_virq(uint32_t virq, evtchn_handler_t handler, void *data) rc = HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq, &op);
        if (rc != 0) {
-               uk_printk("Failed to bind virtual IRQ %d with rc=%d\n", virq, 
rc);
+               uk_pr_err("Failed to bind virtual IRQ %d with rc=%d\n",
+                         virq, rc);
                return -1;
        }
        bind_evtchn(op.port, handler, data);
@@ -169,7 +170,8 @@ evtchn_port_t bind_pirq(uint32_t pirq, int will_share,
rc = HYPERVISOR_event_channel_op(EVTCHNOP_bind_pirq, &op);
        if (rc != 0) {
-               uk_printk("Failed to bind physical IRQ %d with rc=%d\n", pirq, 
rc);
+               uk_pr_err("Failed to bind physical IRQ %d with rc=%d\n",
+                         pirq, rc);
                return -1;
        }
        bind_evtchn(op.port, handler, data);
@@ -210,7 +212,7 @@ void suspend_events(void)
  static void default_handler(evtchn_port_t port, struct __regs *regs __unused,
                            void *ignore __unused)
  {
-       uk_printk("[Port %d] - event received\n", port);
+       uk_pr_info("[Port %d] - event received\n", port);
  }
/* Create a port available to the pal for exchanging notifications.
@@ -232,7 +234,7 @@ int evtchn_alloc_unbound(domid_t pal, evtchn_handler_t 
handler,
        op.remote_dom = pal;
        rc = HYPERVISOR_event_channel_op(EVTCHNOP_alloc_unbound, &op);
        if (rc) {

Add a new line.
-               uk_printk("ERROR: alloc_unbound failed with rc=%d", rc);
+               uk_pr_err("alloc_unbound failed with rc=%d", rc);
                return rc;
        }
@@ -256,7 +258,7 @@ int evtchn_bind_interdomain(domid_t pal, evtchn_port_t remote_port,
        op.remote_port = remote_port;
        rc = HYPERVISOR_event_channel_op(EVTCHNOP_bind_interdomain, &op);
        if (rc) {

Add a new line.
-               uk_printk("ERROR: bind_interdomain failed with rc=%d", rc);
+               uk_pr_err("bind_interdomain failed with rc=%d", rc);
                return rc;
        }

Thanks & Regards
Sharan Santhanam

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

 


Rackspace

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