diff mbox

[2/2] x86: fix printed messages in arch_set_info_hvm_guest

Message ID 20170809101821.50836-2-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Roger Pau Monne Aug. 9, 2017, 10:18 a.m. UTC 
Using gprintk in arch_set_info_hvm_guest doesn't make sense, since
most of the time this function will be called from the toolstack
domain builder or Xen itself in case of Dom0.

Switch to using printk instead.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/hvm/domain.c | 77 +++++++++++++++++++++++++----------------------
 1 file changed, 41 insertions(+), 36 deletions(-)

Comments

Andrew Cooper Aug. 9, 2017, 10:23 a.m. UTC | #1 
On 09/08/17 11:18, Roger Pau Monne wrote:
> Using gprintk in arch_set_info_hvm_guest doesn't make sense, since
> most of the time this function will be called from the toolstack
> domain builder or Xen itself in case of Dom0.
>
> Switch to using printk instead.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  xen/arch/x86/hvm/domain.c | 77 +++++++++++++++++++++++++----------------------
>  1 file changed, 41 insertions(+), 36 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
> index 60474649de..cb4e6cd98f 100644
> --- a/xen/arch/x86/hvm/domain.c
> +++ b/xen/arch/x86/hvm/domain.c
> @@ -24,12 +24,14 @@
>  
>  #include <public/hvm/hvm_vcpu.h>
>  
> -static int check_segment(struct segment_register *reg, enum x86_segment seg)
> +static int check_segment(struct vcpu *v, struct segment_register *reg,
> +                         enum x86_segment seg)
>  {
>  
>      if ( reg->pad != 0 )
>      {
> -        gprintk(XENLOG_ERR, "Segment attribute bits 12-15 are not zero\n");
> +        printk(XENLOG_ERR "%pv Segment attribute bits 12-15 are not zero\n",

These need to retain XENLOG_GUEST so they get suitably ratelimited.

Alternatively, use something like:

gprintk(XENLOG_ERR, "Segment attribute bits 12-15 are not zero for %pv\n")

which gives both the current and target contexts.

~Andrew
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
index 60474649de..cb4e6cd98f 100644
--- a/xen/arch/x86/hvm/domain.c
+++ b/xen/arch/x86/hvm/domain.c
@@ -24,12 +24,14 @@ 
 
 #include <public/hvm/hvm_vcpu.h>
 
-static int check_segment(struct segment_register *reg, enum x86_segment seg)
+static int check_segment(struct vcpu *v, struct segment_register *reg,
+                         enum x86_segment seg)
 {
 
     if ( reg->pad != 0 )
     {
-        gprintk(XENLOG_ERR, "Segment attribute bits 12-15 are not zero\n");
+        printk(XENLOG_ERR "%pv Segment attribute bits 12-15 are not zero\n",
+               v);
         return -EINVAL;
     }
 
@@ -37,7 +39,8 @@  static int check_segment(struct segment_register *reg, enum x86_segment seg)
     {
         if ( seg != x86_seg_ds && seg != x86_seg_es )
         {
-            gprintk(XENLOG_ERR, "Null selector provided for CS, SS or TR\n");
+            printk(XENLOG_ERR "%pv Null selector provided for CS, SS or TR\n",
+                   v);
             return -EINVAL;
         }
         return 0;
@@ -47,26 +50,27 @@  static int check_segment(struct segment_register *reg, enum x86_segment seg)
     {
         if ( reg->s )
         {
-            gprintk(XENLOG_ERR, "Code or data segment provided for TR\n");
+            printk(XENLOG_ERR "%pv Code or data segment provided for TR\n", v);
             return -EINVAL;
         }
 
         if ( reg->type != SYS_DESC_tss_busy )
         {
-            gprintk(XENLOG_ERR, "Non-32-bit-TSS segment provided for TR\n");
+            printk(XENLOG_ERR "%pc Non-32-bit-TSS segment provided for TR\n",
+                   v);
             return -EINVAL;
         }
     }
     else if ( !reg->s )
     {
-        gprintk(XENLOG_ERR,
-                "System segment provided for a code or data segment\n");
+        printk(XENLOG_ERR
+               "%pv System segment provided for a code or data segment\n", v);
         return -EINVAL;
     }
 
     if ( !reg->p )
     {
-        gprintk(XENLOG_ERR, "Non-present segment provided\n");
+        printk(XENLOG_ERR "%pv Non-present segment provided\n", v);
         return -EINVAL;
     }
 
@@ -75,7 +79,7 @@  static int check_segment(struct segment_register *reg, enum x86_segment seg)
     case x86_seg_cs:
         if ( !(reg->type & 0x8) )
         {
-            gprintk(XENLOG_ERR, "Non-code segment provided for CS\n");
+            printk(XENLOG_ERR "%pv Non-code segment provided for CS\n", v);
             return -EINVAL;
         }
         break;
@@ -83,7 +87,7 @@  static int check_segment(struct segment_register *reg, enum x86_segment seg)
     case x86_seg_ss:
         if ( (reg->type & 0x8) || !(reg->type & 0x2) )
         {
-            gprintk(XENLOG_ERR, "Non-writeable segment provided for SS\n");
+            printk(XENLOG_ERR "%pv Non-writeable segment provided for SS\n", v);
             return -EINVAL;
         }
         break;
@@ -92,7 +96,8 @@  static int check_segment(struct segment_register *reg, enum x86_segment seg)
     case x86_seg_es:
         if ( (reg->type & 0x8) && !(reg->type & 0x2) )
         {
-            gprintk(XENLOG_ERR, "Non-readable segment provided for DS or ES\n");
+            printk(XENLOG_ERR
+                   "%pv Non-readable segment provided for DS or ES\n", v);
             return -EINVAL;
         }
         break;
@@ -141,7 +146,7 @@  int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
     /* Set accessed / busy bit for present segments. */                     \
     if ( s.p )                                                              \
         s.type |= (x86_seg_##s != x86_seg_tr ? 1 : 2);                      \
-    check_segment(&s, x86_seg_ ## s); })
+    check_segment(v, &s, x86_seg_ ## s); })
 
         rc = SEG(cs, regs);
         rc |= SEG(ds, regs);
@@ -159,36 +164,36 @@  int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
             limit = (limit << 12) | 0xfff;
         if ( regs->eip > limit )
         {
-            gprintk(XENLOG_ERR, "EIP (%#08x) outside CS limit (%#08x)\n",
-                    regs->eip, limit);
+            printk(XENLOG_ERR "%pv EIP (%#08x) outside CS limit (%#08x)\n",
+                   v, regs->eip, limit);
             return -EINVAL;
         }
 
         if ( ss.dpl != cs.dpl )
         {
-            gprintk(XENLOG_ERR, "SS.DPL (%u) is different than CS.DPL (%u)\n",
-                    ss.dpl, cs.dpl);
+            printk(XENLOG_ERR "%pv SS.DPL (%u) is different than CS.DPL (%u)\n",
+                   v, ss.dpl, cs.dpl);
             return -EINVAL;
         }
 
         if ( ds.p && ds.dpl > cs.dpl )
         {
-            gprintk(XENLOG_ERR, "DS.DPL (%u) is greater than CS.DPL (%u)\n",
-                    ds.dpl, cs.dpl);
+            printk(XENLOG_ERR "%pv DS.DPL (%u) is greater than CS.DPL (%u)\n",
+                   v, ds.dpl, cs.dpl);
             return -EINVAL;
         }
 
         if ( es.p && es.dpl > cs.dpl )
         {
-            gprintk(XENLOG_ERR, "ES.DPL (%u) is greater than CS.DPL (%u)\n",
-                    es.dpl, cs.dpl);
+            printk(XENLOG_ERR "%pv ES.DPL (%u) is greater than CS.DPL (%u)\n",
+                   v, es.dpl, cs.dpl);
             return -EINVAL;
         }
 
         if ( (regs->efer & EFER_LMA) && !(regs->efer & EFER_LME) )
         {
-            gprintk(XENLOG_ERR, "EFER.LMA set without EFER.LME (%#016lx)\n",
-                    regs->efer);
+            printk(XENLOG_ERR "%pv EFER.LMA set without EFER.LME (%#016lx)\n",
+                   v, regs->efer);
             return -EINVAL;
         }
 
@@ -217,29 +222,29 @@  int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
         /* Basic sanity checks. */
         if ( !is_canonical_address(regs->rip) )
         {
-            gprintk(XENLOG_ERR, "RIP contains a non-canonical address (%#lx)\n",
-                    regs->rip);
+            printk(XENLOG_ERR "%pv RIP contains a non-canonical address (%#lx)\n",
+                   v, regs->rip);
             return -EINVAL;
         }
 
         if ( !(regs->cr0 & X86_CR0_PG) )
         {
-            gprintk(XENLOG_ERR, "CR0 doesn't have paging enabled (%#016lx)\n",
-                    regs->cr0);
+            printk(XENLOG_ERR "%pv CR0 doesn't have paging enabled (%#016lx)\n",
+                   v, regs->cr0);
             return -EINVAL;
         }
 
         if ( !(regs->cr4 & X86_CR4_PAE) )
         {
-            gprintk(XENLOG_ERR, "CR4 doesn't have PAE enabled (%#016lx)\n",
-                    regs->cr4);
+            printk(XENLOG_ERR "%pv CR4 doesn't have PAE enabled (%#016lx)\n",
+                   v, regs->cr4);
             return -EINVAL;
         }
 
         if ( !(regs->efer & EFER_LME) )
         {
-            gprintk(XENLOG_ERR, "EFER doesn't have LME enabled (%#016lx)\n",
-                    regs->efer);
+            printk(XENLOG_ERR "%pv EFER doesn't have LME enabled (%#016lx)\n",
+                   v, regs->efer);
             return -EINVAL;
         }
 
@@ -274,16 +279,16 @@  int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
 
     if ( v->arch.hvm_vcpu.guest_cr[4] & ~hvm_cr4_guest_valid_bits(v, 0) )
     {
-        gprintk(XENLOG_ERR, "Bad CR4 value: %#016lx\n",
-                v->arch.hvm_vcpu.guest_cr[4]);
+        printk(XENLOG_ERR "%pv Bad CR4 value: %#016lx\n",
+               v, v->arch.hvm_vcpu.guest_cr[4]);
         return -EINVAL;
     }
 
     errstr = hvm_efer_valid(v, v->arch.hvm_vcpu.guest_efer, -1);
     if ( errstr )
     {
-        gprintk(XENLOG_ERR, "Bad EFER value (%#016lx): %s\n",
-               v->arch.hvm_vcpu.guest_efer, errstr);
+        printk(XENLOG_ERR "%pv Bad EFER value (%#016lx): %s\n",
+               v, v->arch.hvm_vcpu.guest_efer, errstr);
         return -EINVAL;
     }
 
@@ -300,8 +305,8 @@  int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
                                  NULL, P2M_ALLOC);
         if ( !page )
         {
-            gprintk(XENLOG_ERR, "Invalid CR3: %#lx\n",
-                    v->arch.hvm_vcpu.guest_cr[3]);
+            printk(XENLOG_ERR "%pv Invalid CR3: %#lx\n",
+                   v, v->arch.hvm_vcpu.guest_cr[3]);
             return -EINVAL;
         }