* [PATCH 1 of 3] Improve handling of nested page faults
2011-12-01 19:24 [PATCH 0 of 3] Mem access improvements and new type Andres Lagar-Cavilla
@ 2011-12-01 19:24 ` Andres Lagar-Cavilla
2011-12-01 19:24 ` [PATCH 2 of 3] x86/mm: When mem event automatically promotes access rights, let other subsystems know Andres Lagar-Cavilla
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Andres Lagar-Cavilla @ 2011-12-01 19:24 UTC (permalink / raw)
To: xen-devel; +Cc: andres, keir.xen, tim, JBeulich, adin
xen/arch/x86/hvm/hvm.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
Add checks for access type. Be less reliant on implicit semantics.
Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
diff -r 2f8d261e3701 -r d6cc661d770a xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1288,7 +1288,8 @@ int hvm_hap_nested_page_fault(unsigned l
* If this GFN is emulated MMIO or marked as read-only, pass the fault
* to the mmio handler.
*/
- if ( (p2mt == p2m_mmio_dm) || (p2mt == p2m_ram_ro) )
+ if ( (p2mt == p2m_mmio_dm) ||
+ (access_w && (p2mt == p2m_ram_ro)) )
{
if ( !handle_mmio() )
hvm_inject_exception(TRAP_gp_fault, 0, 0);
@@ -1302,7 +1303,7 @@ int hvm_hap_nested_page_fault(unsigned l
p2m_mem_paging_populate(v->domain, gfn);
/* Mem sharing: unshare the page and try again */
- if ( p2mt == p2m_ram_shared )
+ if ( access_w && (p2mt == p2m_ram_shared) )
{
ASSERT(!p2m_is_nestedp2m(p2m));
mem_sharing_unshare_page(p2m->domain, gfn, 0);
@@ -1319,14 +1320,17 @@ int hvm_hap_nested_page_fault(unsigned l
* a large page, we do not change other pages type within that large
* page.
*/
- paging_mark_dirty(v->domain, mfn_x(mfn));
- p2m_change_type(v->domain, gfn, p2m_ram_logdirty, p2m_ram_rw);
+ if ( access_w )
+ {
+ paging_mark_dirty(v->domain, mfn_x(mfn));
+ p2m_change_type(v->domain, gfn, p2m_ram_logdirty, p2m_ram_rw);
+ }
rc = 1;
goto out_put_gfn;
}
/* Shouldn't happen: Maybe the guest was writing to a r/o grant mapping? */
- if ( p2mt == p2m_grant_map_ro )
+ if ( access_w && (p2mt == p2m_grant_map_ro) )
{
gdprintk(XENLOG_WARNING,
"trying to write to read-only grant mapping\n");
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2 of 3] x86/mm: When mem event automatically promotes access rights, let other subsystems know
2011-12-01 19:24 [PATCH 0 of 3] Mem access improvements and new type Andres Lagar-Cavilla
2011-12-01 19:24 ` [PATCH 1 of 3] Improve handling of nested page faults Andres Lagar-Cavilla
@ 2011-12-01 19:24 ` Andres Lagar-Cavilla
2011-12-01 19:24 ` [PATCH 3 of 3] x86/mm: New mem access type to log access Andres Lagar-Cavilla
2011-12-06 20:38 ` [PATCH 0 of 3] Mem access improvements and new type Tim Deegan
3 siblings, 0 replies; 6+ messages in thread
From: Andres Lagar-Cavilla @ 2011-12-01 19:24 UTC (permalink / raw)
To: xen-devel; +Cc: andres, keir.xen, tim, JBeulich, adin
xen/arch/x86/hvm/hvm.c | 20 +++++++++++++++-----
xen/arch/x86/mm/p2m.c | 8 +++++---
xen/include/asm-x86/p2m.h | 9 +++++----
3 files changed, 25 insertions(+), 12 deletions(-)
The mem event fault handler in the p2m can automatically promote the access
rights of a p2m entry. In those scenarios, vcpu's are not paused and they will
immediately retry the faulting instructions. This will generate a second fault
if the underlying entry type requires so (paging, unsharing, pod, etc).
Collapse the two faults into a single one.
Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
diff -r d6cc661d770a -r 7213610b8003 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1205,7 +1205,7 @@ int hvm_hap_nested_page_fault(unsigned l
mfn_t mfn;
struct vcpu *v = current;
struct p2m_domain *p2m;
- int rc;
+ int rc, fall_through = 0;
/* On Nested Virtualization, walk the guest page table.
* If this succeeds, all is fine.
@@ -1278,9 +1278,15 @@ int hvm_hap_nested_page_fault(unsigned l
if ( violation )
{
- p2m_mem_access_check(gpa, gla_valid, gla, access_r, access_w, access_x);
- rc = 1;
- goto out_put_gfn;
+ if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
+ access_w, access_x) )
+ {
+ fall_through = 1;
+ } else {
+ /* Rights not promoted, vcpu paused, work here is done */
+ rc = 1;
+ goto out_put_gfn;
+ }
}
}
@@ -1339,7 +1345,11 @@ int hvm_hap_nested_page_fault(unsigned l
goto out_put_gfn;
}
- rc = 0;
+ /* If we fell through, the vcpu will retry now that access restrictions have
+ * been removed. It may fault again if the p2m entry type still requires so.
+ * Otherwise, this is an error condition. */
+ rc = fall_through;
+
out_put_gfn:
put_gfn(p2m->domain, gfn);
return rc;
diff -r d6cc661d770a -r 7213610b8003 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1107,7 +1107,7 @@ void p2m_mem_paging_resume(struct domain
mem_event_unpause_vcpus(d, &d->mem_event->paging);
}
-void p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned long gla,
+bool_t p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned long gla,
bool_t access_r, bool_t access_w, bool_t access_x)
{
struct vcpu *v = current;
@@ -1127,7 +1127,7 @@ void p2m_mem_access_check(unsigned long
{
p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt, p2m_access_rw);
p2m_unlock(p2m);
- return;
+ return 1;
}
p2m_unlock(p2m);
@@ -1147,9 +1147,10 @@ void p2m_mem_access_check(unsigned long
p2m_lock(p2m);
p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt, p2m_access_rwx);
p2m_unlock(p2m);
+ return 1;
}
- return;
+ return 0;
}
memset(&req, 0, sizeof(req));
@@ -1173,6 +1174,7 @@ void p2m_mem_access_check(unsigned long
(void)mem_event_put_request(d, &d->mem_event->access, &req);
/* VCPU paused */
+ return 0;
}
void p2m_mem_access_resume(struct domain *d)
diff -r d6cc661d770a -r 7213610b8003 xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -491,8 +491,9 @@ static inline void p2m_mem_paging_popula
#ifdef __x86_64__
/* Send mem event based on the access (gla is -1ull if not available). Handles
- * the rw2rx conversion */
-void p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned long gla,
+ * the rw2rx conversion. Boolean return value indicates if access rights have
+ * been promoted with no underlying vcpu pause. */
+bool_t p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned long gla,
bool_t access_r, bool_t access_w, bool_t access_x);
/* Resumes the running of the VCPU, restarting the last instruction */
void p2m_mem_access_resume(struct domain *d);
@@ -508,10 +509,10 @@ int p2m_get_mem_access(struct domain *d,
hvmmem_access_t *access);
#else
-static inline void p2m_mem_access_check(unsigned long gpa, bool_t gla_valid,
+static inline bool_t p2m_mem_access_check(unsigned long gpa, bool_t gla_valid,
unsigned long gla, bool_t access_r,
bool_t access_w, bool_t access_x)
-{ }
+{ return 1; }
static inline int p2m_set_mem_access(struct domain *d,
unsigned long start_pfn,
uint32_t nr, hvmmem_access_t access)
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 3 of 3] x86/mm: New mem access type to log access
2011-12-01 19:24 [PATCH 0 of 3] Mem access improvements and new type Andres Lagar-Cavilla
2011-12-01 19:24 ` [PATCH 1 of 3] Improve handling of nested page faults Andres Lagar-Cavilla
2011-12-01 19:24 ` [PATCH 2 of 3] x86/mm: When mem event automatically promotes access rights, let other subsystems know Andres Lagar-Cavilla
@ 2011-12-01 19:24 ` Andres Lagar-Cavilla
2011-12-06 20:38 ` [PATCH 0 of 3] Mem access improvements and new type Tim Deegan
3 siblings, 0 replies; 6+ messages in thread
From: Andres Lagar-Cavilla @ 2011-12-01 19:24 UTC (permalink / raw)
To: xen-devel; +Cc: andres, keir.xen, tim, JBeulich, adin
xen/arch/x86/hvm/hvm.c | 1 +
xen/arch/x86/mm/p2m-ept.c | 1 +
xen/arch/x86/mm/p2m.c | 30 +++++++++++++++++++++---------
xen/include/asm-x86/p2m.h | 3 +++
xen/include/public/hvm/hvm_op.h | 3 +++
5 files changed, 29 insertions(+), 9 deletions(-)
This patch adds a new p2m access type, n2rwx. It allows for implement a "log
access" mode in the hypervisor, aking to log dirty but for all types of
accesses. Faults caused by this access mode automatically promote the
access rights of the ofending p2m entry, place the event in the ring, and
let the vcpu keep on executing.
Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
Signed-off-by: Adin Scannell <adin@scannell.ca>
diff -r 7213610b8003 -r 92a379fcef54 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1250,6 +1250,7 @@ int hvm_hap_nested_page_fault(unsigned l
switch (p2ma)
{
case p2m_access_n:
+ case p2m_access_n2rwx:
default:
violation = access_r || access_w || access_x;
break;
diff -r 7213610b8003 -r 92a379fcef54 xen/arch/x86/mm/p2m-ept.c
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -111,6 +111,7 @@ static void ept_p2m_type_to_flags(ept_en
switch (access)
{
case p2m_access_n:
+ case p2m_access_n2rwx:
entry->r = entry->w = entry->x = 0;
break;
case p2m_access_r:
diff -r 7213610b8003 -r 92a379fcef54 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1129,6 +1129,11 @@ bool_t p2m_mem_access_check(unsigned lon
p2m_unlock(p2m);
return 1;
}
+ else if ( p2ma == p2m_access_n2rwx )
+ {
+ ASSERT(access_w || access_r || access_x);
+ p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt, p2m_access_rwx);
+ }
p2m_unlock(p2m);
/* Otherwise, check if there is a memory event listener, and send the message along */
@@ -1143,10 +1148,13 @@ bool_t p2m_mem_access_check(unsigned lon
}
else
{
- /* A listener is not required, so clear the access restrictions */
- p2m_lock(p2m);
- p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt, p2m_access_rwx);
- p2m_unlock(p2m);
+ if ( p2ma != p2m_access_n2rwx )
+ {
+ /* A listener is not required, so clear the access restrictions */
+ p2m_lock(p2m);
+ p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt, p2m_access_rwx);
+ p2m_unlock(p2m);
+ }
return 1;
}
@@ -1157,9 +1165,12 @@ bool_t p2m_mem_access_check(unsigned lon
req.type = MEM_EVENT_TYPE_ACCESS;
req.reason = MEM_EVENT_REASON_VIOLATION;
- /* Pause the current VCPU unconditionally */
- vcpu_pause_nosync(v);
- req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED;
+ /* Pause the current VCPU */
+ if ( p2ma != p2m_access_n2rwx )
+ {
+ vcpu_pause_nosync(v);
+ req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED;
+ }
/* Send request to mem event */
req.gfn = gfn;
@@ -1173,8 +1184,8 @@ bool_t p2m_mem_access_check(unsigned lon
req.vcpu_id = v->vcpu_id;
(void)mem_event_put_request(d, &d->mem_event->access, &req);
- /* VCPU paused */
- return 0;
+ /* VCPU may be paused, return whether we promoted automatically */
+ return (p2ma == p2m_access_n2rwx);
}
void p2m_mem_access_resume(struct domain *d)
@@ -1218,6 +1229,7 @@ int p2m_set_mem_access(struct domain *d,
p2m_access_wx,
p2m_access_rwx,
p2m_access_rx2rw,
+ p2m_access_n2rwx,
p2m->default_access,
};
diff -r 7213610b8003 -r 92a379fcef54 xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -108,6 +108,9 @@ typedef enum {
p2m_access_wx = 6,
p2m_access_rwx = 7,
p2m_access_rx2rw = 8, /* Special: page goes from RX to RW on write */
+ p2m_access_n2rwx = 9, /* Special: page goes from N to RWX on access, *
+ * generates an event but does not pause the
+ * vcpu */
/* NOTE: Assumed to be only 4 bits right now */
} p2m_access_t;
diff -r 7213610b8003 -r 92a379fcef54 xen/include/public/hvm/hvm_op.h
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -174,6 +174,9 @@ typedef enum {
HVMMEM_access_rwx,
HVMMEM_access_rx2rw, /* Page starts off as r-x, but automatically
* change to r-w on a write */
+ HVMMEM_access_n2rwx, /* Log access: starts off as n, automatically
+ * goes to rwx, generating an event without
+ * pausing the vcpu */
HVMMEM_access_default /* Take the domain default */
} hvmmem_access_t;
/* Notify that a region of memory is to have specific access types */
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0 of 3] Mem access improvements and new type
2011-12-01 19:24 [PATCH 0 of 3] Mem access improvements and new type Andres Lagar-Cavilla
` (2 preceding siblings ...)
2011-12-01 19:24 ` [PATCH 3 of 3] x86/mm: New mem access type to log access Andres Lagar-Cavilla
@ 2011-12-06 20:38 ` Tim Deegan
2011-12-06 21:05 ` Andres Lagar-Cavilla
3 siblings, 1 reply; 6+ messages in thread
From: Tim Deegan @ 2011-12-06 20:38 UTC (permalink / raw)
To: Andres Lagar-Cavilla; +Cc: andres, xen-devel, keir.xen, JBeulich, adin
At 14:24 -0500 on 01 Dec (1322749496), Andres Lagar-Cavilla wrote:
> We improve the handling of hap faults when both type and access
> restrictions are present.
>
> We also add a new p2m access type, n2rwx. It allows for implement a "log
> access" mode in the hypervisor, aking to log dirty but for all types of
> accesses. Faults caused by this access mode automatically promote the
> access rights of the ofending p2m entry, place the event in the ring, and
> let the vcpu keep on executing.
Applied 1/3; thanks.
The other two don't apply cleanly to xen-unstable. have you got some
other patches in you local environment that touch the same parts of
p2m.c?
Tim.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0 of 3] Mem access improvements and new type
2011-12-06 20:38 ` [PATCH 0 of 3] Mem access improvements and new type Tim Deegan
@ 2011-12-06 21:05 ` Andres Lagar-Cavilla
0 siblings, 0 replies; 6+ messages in thread
From: Andres Lagar-Cavilla @ 2011-12-06 21:05 UTC (permalink / raw)
To: Tim Deegan; +Cc: andres, xen-devel, keir.xen, jbeulich, adin
> At 14:24 -0500 on 01 Dec (1322749496), Andres Lagar-Cavilla wrote:
>> We improve the handling of hap faults when both type and access
>> restrictions are present.
>>
>> We also add a new p2m access type, n2rwx. It allows for implement a "log
>> access" mode in the hypervisor, aking to log dirty but for all types of
>> accesses. Faults caused by this access mode automatically promote the
>> access rights of the ofending p2m entry, place the event in the ring,
>> and
>> let the vcpu keep on executing.
>
> Applied 1/3; thanks.
>
> The other two don't apply cleanly to xen-unstable. have you got some
> other patches in you local environment that touch the same parts of
> p2m.c?
I don't know. Rebased, seems clean, resent just now.
Thanks a bunch
Andres
>
> Tim.
>
^ permalink raw reply [flat|nested] 6+ messages in thread