add HVMOP_get_mem_type hvmop

PATCH Superseded Olaf Hering 3 participants 9 messages May 3, 2011 – May 18, 2011

Share & tools

Olaf Hering

author

2011-05-03 13:11 UTC #0 lore raw

PATCH 1/1 add HVMOP_get_mem_type hvmop

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1304427973 -7200
# Node ID e7356373147a21df7280ce55e05ff1057f1a0cc9
# Parent 10f27b8b3d63959c7a8e15299a7a398b7ff7f230
The balloon driver in the guest frees guest pages and marks them as mmio. When the kernel crashes and the crash kernel attempts to read the oldmem via /proc/vmcore a read from ballooned pages will generate 100% load in dom0 because Xen asks qemu-dm for the page content. Since the reads come in as 8byte requests each ballooned page is tried 512 times.

Add a new hvmop HVMOP_get_mem_type to return the hvmmem_type_t for the given pfn. Pages which are neither ram or mmio will be HVMMEM_mmio_dm. This interface enables the crash kernel to skip ballooned pages.

Signed-off-by Olaf Hering <[email protected]>

xen/arch/ia64/vmx/vmx_hypercall.c +1 -0

diff -r 10f27b8b3d63 -r e7356373147a xen/arch/ia64/vmx/vmx_hypercall.c
@@ -217,6 +217,7 @@ do_hvm_op(unsigned long op, XEN_GUEST_HA
         break;
     }
 
+    case HVMOP_get_mem_type:
     case HVMOP_set_mem_type:
     case HVMOP_set_mem_access:
     case HVMOP_get_mem_access:

xen/arch/x86/hvm/hvm.c +31 -0

diff -r 10f27b8b3d63 -r e7356373147a xen/arch/x86/hvm/hvm.c
@@ -3676,6 +3676,37 @@ long do_hvm_op(unsigned long op, XEN_GUE
         break;
     }
 
+    case HVMOP_get_mem_type:
+    {
+        struct xen_hvm_get_mem_type a;
+        struct domain *d;
+        p2m_type_t t;
+
+        if ( copy_from_guest(&a, arg, 1) )
+            return -EFAULT;
+
+        rc = rcu_lock_remote_target_domain_by_id(a.domid, &d);
+        if ( rc != 0 )
+            return rc;
+
+        rc = -EINVAL;
+        if ( is_hvm_domain(d) )
+        {
+            gfn_to_mfn_unshare(p2m_get_hostp2m(d), a.pfn, &t, 0);
+            if ( p2m_is_mmio(t) )
+                a.mem_type =  HVMMEM_mmio_dm;
+            else if ( p2m_is_readonly(t) )
+                a.mem_type =  HVMMEM_ram_ro;
+            else if ( p2m_is_ram(t) )
+                a.mem_type =  HVMMEM_ram_rw;
+            else
+                a.mem_type =  HVMMEM_mmio_dm;
+            rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
+        }
+        rcu_unlock_domain(d);
+        break;
+    }
+
     case HVMOP_set_mem_type:
     {
         struct xen_hvm_set_mem_type a;

xen/include/public/hvm/hvm_op.h +20 -5

diff -r 10f27b8b3d63 -r e7356373147a xen/include/public/hvm/hvm_op.h
@@ -76,6 +76,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_
 /* Flushes all VCPU TLBs: @arg must be NULL. */
 #define HVMOP_flush_tlbs          5
 
+typedef enum {
+    HVMMEM_ram_rw,             /* Normal read/write guest RAM */
+    HVMMEM_ram_ro,             /* Read-only; writes are discarded */
+    HVMMEM_mmio_dm,            /* Reads and write go to the device model */
+} hvmmem_type_t;
+
 /* Following tools-only interfaces may change in future. */
 #if defined(__XEN__) || defined(__XEN_TOOLS__)

@@ -109,11 +115,6 @@ typedef struct xen_hvm_modified_memory x
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_modified_memory_t);
 
 #define HVMOP_set_mem_type    8
-typedef enum {
-    HVMMEM_ram_rw,             /* Normal read/write guest RAM */
-    HVMMEM_ram_ro,             /* Read-only; writes are discarded */
-    HVMMEM_mmio_dm,            /* Reads and write go to the device model */
-} hvmmem_type_t;
 /* Notify that a region of memory is to be treated in a specific way. */
 struct xen_hvm_set_mem_type {
     /* Domain to be updated. */

@@ -223,6 +224,20 @@ struct xen_hvm_inject_trap {
 typedef struct xen_hvm_inject_trap xen_hvm_inject_trap_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_inject_trap_t);
 
+#define HVMOP_get_mem_type    15
+/* Return hvmmem_type_t for the specified pfn. */
+struct xen_hvm_get_mem_type {
+    /* Domain to be queried. */
+    domid_t domid;
+    /* OUT variable. */
+    uint8_t mem_type;
+    /* IN variable. */
+    uint64_t pfn;
+};
+typedef struct xen_hvm_get_mem_type xen_hvm_get_mem_type_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvm_get_mem_type_t);
+
+
 #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
 
 #endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */

Tim Deegan

2011-05-03 13:30 UTC #1 lore raw

Hi,

Quoted 7 lines · Olaf Hering

diff -r 10f27b8b3d63 -r e7356373147a xen/arch/x86/hvm/hvm.c
xen/arch/x86/hvm/hvm.c +0 -0
@@ -3676,6 +3676,37 @@ long do_hvm_op(unsigned long op, XEN_GUE
         break;
     }
 

xen/arch/x86/hvm/hvm.c long do_hvm_op(unsigned long op, XEN_GUE · line 3688

Quoted 10 lines

xen/arch/x86/hvm/hvm.c +10 -0

+    case HVMOP_get_mem_type:
+    {
+        struct xen_hvm_get_mem_type a;
+        struct domain *d;
+        p2m_type_t t;
+
+        if ( copy_from_guest(&a, arg, 1) )
+            return -EFAULT;
+
+        rc = rcu_lock_remote_target_domain_by_id(a.domid, &d);

I thought this call was intended to be used from inside the guest in question. rcu_lock_remote_target_domain_by_id() explicitly refuses to let a domain operate on itself.

Quoted 28 lines

xen/arch/x86/hvm/hvm.c +21 -0

+        if ( rc != 0 )
+            return rc;
+
+        rc = -EINVAL;
+        if ( is_hvm_domain(d) )
+        {
+            gfn_to_mfn_unshare(p2m_get_hostp2m(d), a.pfn, &t, 0);
+            if ( p2m_is_mmio(t) )
+                a.mem_type =  HVMMEM_mmio_dm;
+            else if ( p2m_is_readonly(t) )
+                a.mem_type =  HVMMEM_ram_ro;
+            else if ( p2m_is_ram(t) )
+                a.mem_type =  HVMMEM_ram_rw;
+            else
+                a.mem_type =  HVMMEM_mmio_dm;
+            rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
+        }
+        rcu_unlock_domain(d);
+        break;
+    }
+
     case HVMOP_set_mem_type:
     {
         struct xen_hvm_set_mem_type a;

diff -r 10f27b8b3d63 -r e7356373147a xen/include/public/hvm/hvm_op.h

xen/include/public/hvm/hvm_op.h DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_ · line 84

Quoted 9 lines

xen/include/public/hvm/hvm_op.h +6 -0

@@ -76,6 +76,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_
 /* Flushes all VCPU TLBs: @arg must be NULL. */
 #define HVMOP_flush_tlbs          5
 
+typedef enum {
+    HVMMEM_ram_rw,             /* Normal read/write guest RAM */
+    HVMMEM_ram_ro,             /* Read-only; writes are discarded */
+    HVMMEM_mmio_dm,            /* Reads and write go to the device model */
+} hvmmem_type_t;
+

This is now outside the #ifdef, when both of its users are inside it. If that wasn't deliberate, please put it back.

Quoted 18 lines

xen/include/public/hvm/hvm_op.h +0 -5

 /* Following tools-only interfaces may change in future. */
 #if defined(__XEN__) || defined(__XEN_TOOLS__)

@@ -109,11 +115,6 @@ typedef struct xen_hvm_modified_memory x
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_modified_memory_t);
 
 #define HVMOP_set_mem_type    8
-typedef enum {
-    HVMMEM_ram_rw,             /* Normal read/write guest RAM */
-    HVMMEM_ram_ro,             /* Read-only; writes are discarded */
-    HVMMEM_mmio_dm,            /* Reads and write go to the device model */
-} hvmmem_type_t;
 /* Notify that a region of memory is to be treated in a specific way. */
 struct xen_hvm_set_mem_type {
     /* Domain to be updated. */

@@ -223,6 +224,20 @@ struct xen_hvm_inject_trap {
 typedef struct xen_hvm_inject_trap xen_hvm_inject_trap_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_inject_trap_t);

xen/include/public/hvm/hvm_op.h struct xen_hvm_inject_trap { · line 235

Quoted 10 lines

xen/include/public/hvm/hvm_op.h +9 -0

 
+#define HVMOP_get_mem_type    15
+/* Return hvmmem_type_t for the specified pfn. */
+struct xen_hvm_get_mem_type {
+    /* Domain to be queried. */
+    domid_t domid;
+    /* OUT variable. */
+    uint8_t mem_type;
+    /* IN variable. */
+    uint64_t pfn;

This structure will be laid out differently on 32-bit and 64-bit builds. :( Also, since the _set operation uses a 16-bit variable for the type, you might as well do the same here.

Cheers,

Tim.

Signature · 3 lines

Tim Deegan <[email protected]>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Olaf Hering

author

2011-05-03 13:59 UTC #2 lore raw

Quoted 5 lines · Tim Deegan

Quoted 1 line
+ rc = rcu_lock_remote_target_domain_by_id(a.domid, &d);
I thought this call was intended to be used from inside the guest in question. rcu_lock_remote_target_domain_by_id() explicitly refuses to let a domain operate on itself.

Hmm, I have to test it with xen-unstable too. I did copy&paste from other parts of the file. 4.0 used rcu_lock_target_domain_by_id, so can I use that function?

Quoted 9 lines

xen/include/public/hvm/hvm_op.h DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_ · line 84
Quoted 6 lines
xen/include/public/hvm/hvm_op.h +6 -0
+typedef enum {
+    HVMMEM_ram_rw,             /* Normal read/write guest RAM */
+    HVMMEM_ram_ro,             /* Read-only; writes are discarded */
+    HVMMEM_mmio_dm,            /* Reads and write go to the device model */
+} hvmmem_type_t;
+
This is now outside the #ifdef, when both of its users are inside it. If that wasn't deliberate, please put it back.

Should HVMOP_get_mem_type use hvmmem_type_t? Its outside the ifdef, otherwise unmodified_drivers/linux-2.6/platform-pci/platform-pci.c does not get the define.

Quoted 19 lines

/* Following tools-only interfaces may change in future. */ #if defined(__XEN__) || defined(__XEN_TOOLS__)

xen/include/public/hvm/hvm_op.h +0 -5

@@ -109,11 +115,6 @@ typedef struct xen_hvm_modified_memory x
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_modified_memory_t);
 
 #define HVMOP_set_mem_type    8
-typedef enum {
-    HVMMEM_ram_rw,             /* Normal read/write guest RAM */
-    HVMMEM_ram_ro,             /* Read-only; writes are discarded */
-    HVMMEM_mmio_dm,            /* Reads and write go to the device model */
-} hvmmem_type_t;
 /* Notify that a region of memory is to be treated in a specific way. */
 struct xen_hvm_set_mem_type {
     /* Domain to be updated. */

@@ -223,6 +224,20 @@ struct xen_hvm_inject_trap {
 typedef struct xen_hvm_inject_trap xen_hvm_inject_trap_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_inject_trap_t);

Quoted 13 lines

xen/include/public/hvm/hvm_op.h struct xen_hvm_inject_trap { · line 235
Quoted 9 lines
xen/include/public/hvm/hvm_op.h +9 -0
+#define HVMOP_get_mem_type    15
+/* Return hvmmem_type_t for the specified pfn. */
+struct xen_hvm_get_mem_type {
+    /* Domain to be queried. */
+    domid_t domid;
+    /* OUT variable. */
+    uint8_t mem_type;
+    /* IN variable. */
+    uint64_t pfn;
This structure will be laid out differently on 32-bit and 64-bit builds. :( Also, since the _set operation uses a 16-bit variable for the type, you might as well do the same here.

Good point. Should I use the same padding as in xen_hvm_pagetable_dying?

struct xen_hvm_get_mem_type {
    /* Domain to be queried. */
    domid_t domid;
    /* OUT variable. */
    uint16_t mem_type;
    uint16_t pad[2];
    /* IN variable. */
    uint64_t pfn;
};

Olaf

Tim Deegan

2011-05-03 14:07 UTC #3 lore raw

Quoted 11 lines · Olaf Hering

Quoted 5 lines · Tim Deegan
Quoted 1 line
+ rc = rcu_lock_remote_target_domain_by_id(a.domid, &d);
I thought this call was intended to be used from inside the guest in question. rcu_lock_remote_target_domain_by_id() explicitly refuses to let a domain operate on itself.
Hmm, I have to test it with xen-unstable too. I did copy&paste from other parts of the file. 4.0 used rcu_lock_target_domain_by_id, so can I use that function?

Yes; I think that's the right one for you. It's the same except for the check that the target isn't the caller.

Quoted 11 lines

Quoted 9 lines
xen/include/public/hvm/hvm_op.h DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_ · line 84
Quoted 6 lines
xen/include/public/hvm/hvm_op.h +6 -0
+typedef enum {
+    HVMMEM_ram_rw,             /* Normal read/write guest RAM */
+    HVMMEM_ram_ro,             /* Read-only; writes are discarded */
+    HVMMEM_mmio_dm,            /* Reads and write go to the device model */
+} hvmmem_type_t;
+
This is now outside the #ifdef, when both of its users are inside it. If that wasn't deliberate, please put it back.
Should HVMOP_get_mem_type use hvmmem_type_t? Its outside the ifdef,

Looks to me like it's inside; at least, inside an equivalent one. Did you compile-test your kernel against this patch?

Quoted 2 lines

otherwise unmodified_drivers/linux-2.6/platform-pci/platform-pci.c does not get the define.

OK; as long as both the typedef and its user are outside the ifdef, that's fine.

Quoted 16 lines

Quoted 5 lines
Quoted 5 lines
xen/include/public/hvm/hvm_op.h +5 -0
+#define HVMOP_get_mem_type    15
+/* Return hvmmem_type_t for the specified pfn. */
+struct xen_hvm_get_mem_type {
+    /* Domain to be queried. */
+    domid_t domid;
Quoted 8 lines
xen/include/public/hvm/hvm_op.h struct xen_hvm_inject_trap { · line 235
Quoted 4 lines
xen/include/public/hvm/hvm_op.h +4 -0
+    /* OUT variable. */
+    uint8_t mem_type;
+    /* IN variable. */
+    uint64_t pfn;
This structure will be laid out differently on 32-bit and 64-bit builds. :( Also, since the _set operation uses a 16-bit variable for the type, you might as well do the same here.
Good point. Should I use the same padding as in xen_hvm_pagetable_dying?

Quoted 9 lines

struct xen_hvm_get_mem_type {
    /* Domain to be queried. */
    domid_t domid;
    /* OUT variable. */
    uint16_t mem_type;
    uint16_t pad[2];
    /* IN variable. */
    uint64_t pfn;
};

Yes, that's fine.

Tim.

Signature · 3 lines

Tim Deegan <[email protected]>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Olaf Hering

author

2011-05-04 10:34 UTC #4 lore raw

PATCH v2 1/1 add HVMOP_get_mem_type hvmop

In reply to Olaf Hering

The balloon driver in the guest frees guest pages and marks them as mmio. When the kernel crashes and the crash kernel attempts to read the oldmem via /proc/vmcore a read from ballooned pages will generate 100% load in dom0 because Xen asks qemu-dm for the page content. Since the reads come in as 8byte requests each ballooned page is tried 512 times.

Signed-off-by Olaf Hering <[email protected]>

xen/arch/ia64/vmx/vmx_hypercall.c +1 -0

diff -r 10f27b8b3d63 xen/arch/ia64/vmx/vmx_hypercall.c
@@ -217,6 +217,7 @@ do_hvm_op(unsigned long op, XEN_GUEST_HA
         break;
     }
 
+    case HVMOP_get_mem_type:
     case HVMOP_set_mem_type:
     case HVMOP_set_mem_access:
     case HVMOP_get_mem_access:

xen/arch/x86/hvm/hvm.c +31 -0

diff -r 10f27b8b3d63 xen/arch/x86/hvm/hvm.c
@@ -3676,6 +3676,37 @@ long do_hvm_op(unsigned long op, XEN_GUE
         break;
     }
 
+    case HVMOP_get_mem_type:
+    {
+        struct xen_hvm_get_mem_type a;
+        struct domain *d;
+        p2m_type_t t;
+
+        if ( copy_from_guest(&a, arg, 1) )
+            return -EFAULT;
+
+        rc = rcu_lock_target_domain_by_id(a.domid, &d);
+        if ( rc != 0 )
+            return rc;
+
+        rc = -EINVAL;
+        if ( is_hvm_domain(d) )
+        {
+            gfn_to_mfn_unshare(p2m_get_hostp2m(d), a.pfn, &t, 0);
+            if ( p2m_is_mmio(t) )
+                a.mem_type =  HVMMEM_mmio_dm;
+            else if ( p2m_is_readonly(t) )
+                a.mem_type =  HVMMEM_ram_ro;
+            else if ( p2m_is_ram(t) )
+                a.mem_type =  HVMMEM_ram_rw;
+            else
+                a.mem_type =  HVMMEM_mmio_dm;
+            rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
+        }
+        rcu_unlock_domain(d);
+        break;
+    }
+
     case HVMOP_set_mem_type:
     {
         struct xen_hvm_set_mem_type a;

xen/include/public/hvm/hvm_op.h +20 -5

diff -r 10f27b8b3d63 xen/include/public/hvm/hvm_op.h
@@ -76,6 +76,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_
 /* Flushes all VCPU TLBs: @arg must be NULL. */
 #define HVMOP_flush_tlbs          5
 
+typedef enum {
+    HVMMEM_ram_rw,             /* Normal read/write guest RAM */
+    HVMMEM_ram_ro,             /* Read-only; writes are discarded */
+    HVMMEM_mmio_dm,            /* Reads and write go to the device model */
+} hvmmem_type_t;
+
 /* Following tools-only interfaces may change in future. */
 #if defined(__XEN__) || defined(__XEN_TOOLS__)

@@ -109,11 +115,6 @@ typedef struct xen_hvm_modified_memory x
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_modified_memory_t);
 
 #define HVMOP_set_mem_type    8
-typedef enum {
-    HVMMEM_ram_rw,             /* Normal read/write guest RAM */
-    HVMMEM_ram_ro,             /* Read-only; writes are discarded */
-    HVMMEM_mmio_dm,            /* Reads and write go to the device model */
-} hvmmem_type_t;
 /* Notify that a region of memory is to be treated in a specific way. */
 struct xen_hvm_set_mem_type {
     /* Domain to be updated. */

@@ -225,4 +226,18 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_inject_t
 
 #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
 
+#define HVMOP_get_mem_type    15
+/* Return hvmmem_type_t for the specified pfn. */
+struct xen_hvm_get_mem_type {
+    /* Domain to be queried. */
+    domid_t domid;
+    /* OUT variable. */
+    uint16_t mem_type;
+    uint16_t pad[2]; /* align next field on 8-byte boundary */
+    /* IN variable. */
+    uint64_t pfn;
+};
+typedef struct xen_hvm_get_mem_type xen_hvm_get_mem_type_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvm_get_mem_type_t);
+
 #endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */

Tim Deegan

Applied

2011-05-04 12:40 UTC #5 lore raw

Quoted 3 lines · Olaf Hering

Add a new hvmop HVMOP_get_mem_type to return the hvmmem_type_t for the given pfn. Pages which are neither ram or mmio will be HVMMEM_mmio_dm. This interface enables the crash kernel to skip ballooned pages.

Applied; thanks.

Tim.

Signature · 3 lines

Tim Deegan <[email protected]>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Olaf Hering

author

2011-05-17 09:00 UTC #6 lore raw

Quoted 6 lines · Tim Deegan

Quoted 3 lines · Olaf Hering
Add a new hvmop HVMOP_get_mem_type to return the hvmmem_type_t for the given pfn. Pages which are neither ram or mmio will be HVMMEM_mmio_dm. This interface enables the crash kernel to skip ballooned pages.
Applied; thanks.

Tim.

if this new hvmop interface is now finalized, could this be applied to 4.1 as well, and the header file change applied to the linux-2.6.18.hg and also mainline?

Olaf

Tim Deegan

2011-05-18 09:23 UTC #7 lore raw

Quoted 7 lines · Olaf Hering

Quoted 5 lines · Tim Deegan
Quoted 3 lines · Olaf Hering
Add a new hvmop HVMOP_get_mem_type to return the hvmmem_type_t for the given pfn. Pages which are neither ram or mmio will be HVMMEM_mmio_dm. This interface enables the crash kernel to skip ballooned pages.

Quoted 7 lines

Quoted 1 line
Applied; thanks.
Tim.
if this new hvmop interface is now finalized, could this be applied to 4.1 as well, and the header file change applied to the linux-2.6.18.hg and also mainline?

That's a question for Keir, who maintains the 4.1 tree. But we don't usually backport new features to the older trees.

Tim.

Signature · 3 lines

Tim Deegan <[email protected]>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Keir Fraser

2011-05-18 10:10 UTC #8 lore raw

Quoted 6 lines · Tim Deegan

Quoted 3 lines
if this new hvmop interface is now finalized, could this be applied to 4.1 as well, and the header file change applied to the linux-2.6.18.hg and also mainline?
That's a question for Keir, who maintains the 4.1 tree. But we don't usually backport new features to the older trees.

This one looks harmless enough, I'll backport it.

-- Keir