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

Re: [Minios-devel] [UNIKRAFT PATCH v2 05/10] plat/*: Replace uk_printk() with uk_pr_*() equivalents



Hello Simon,

This patch looks fine with the exception of one minor comment. I am fine if we can accept this patch and fix this minor issue while upstreaming this patch.

Reviewed-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>

On 09/27/2018 02:41 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/xen/events.c       | 24 +++++++++++++-----------
  4 files changed, 27 insertions(+), 26 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/xen/events.c b/plat/xen/events.c
index 1709b90..6721985 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) {

Not sure if this is an error. The only place where we use this function we ignore the return value.
-               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) {
-               uk_printk("ERROR: alloc_unbound failed with rc=%d", rc);
+               uk_pr_err("alloc_unbound failed with rc=%d\n", 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) {
-               uk_printk("ERROR: bind_interdomain failed with rc=%d", rc);
+               uk_pr_err("bind_interdomain failed with rc=%d\n", 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®.