* [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
@ 2014-08-07 19:47 Tamas K Lengyel
2014-08-07 19:47 ` [PATCH v2 2/2] tools/xen-access: Print gla valid/fault information Tamas K Lengyel
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Tamas K Lengyel @ 2014-08-07 19:47 UTC (permalink / raw)
To: xen-devel
Cc: kevin.tian, ian.campbell, stefano.stabellini, jun.nakajima,
eddie.dong, ian.jackson, Aravind.Gopalakrishnan,
suravee.suthikulpanit, Tamas K Lengyel, boris.ostrovsky
On Intel EPT the exit qualification generated by a violation also includes a bit (EPT_GLA_FAULT) which describes the following information:
Set if the access causing the EPT violation is to a guest-physical address that is the translation of a linear address. Clear if the access causing the EPT violation is to a paging-structure entry as part of a page walk or the update of an accessed or dirty bit.
For more information see Table 27-7 in the Intel SDM.
This patch extends the mem_event system to deliver this extra information, which could be useful for determining the cause of a violation.
v2: Split gla_fault into fault_in_gpt and fault_gla to be more compatible with the AMD implementation.
Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
---
xen/arch/x86/hvm/hvm.c | 8 ++++++--
xen/arch/x86/hvm/svm/svm.c | 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++++++-
xen/arch/x86/mm/p2m.c | 5 ++++-
xen/include/asm-x86/hvm/hvm.h | 5 ++++-
xen/include/asm-x86/p2m.h | 3 ++-
xen/include/public/mem_event.h | 4 +++-
7 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index e834406..d7b5e2b 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned long cr2)
int hvm_hap_nested_page_fault(paddr_t gpa,
bool_t gla_valid,
unsigned long gla,
+ bool_t fault_in_gpt,
+ bool_t fault_gla,
bool_t access_r,
bool_t access_w,
bool_t access_x)
@@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
if ( violation )
{
- if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
- access_w, access_x, &req_ptr) )
+ if ( p2m_mem_access_check(gpa, gla_valid, gla,
+ fault_in_gpt, fault_gla,
+ access_r, access_w, access_x,
+ &req_ptr) )
{
fall_through = 1;
} else {
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 76616ac..9e35e7a 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1403,7 +1403,7 @@ static void svm_do_nested_pgfault(struct vcpu *v,
p2m_access_t p2ma;
struct p2m_domain *p2m = NULL;
- ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul,
+ ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul, 0, 0,
1, /* All NPFs count as reads */
npfec & PFEC_write_access,
npfec & PFEC_insn_fetch);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 2caa04a..7ebf289 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2353,6 +2353,7 @@ static void ept_handle_violation(unsigned long qualification, paddr_t gpa)
p2m_type_t p2mt;
int ret;
struct domain *d = current->domain;
+ bool_t fault_in_gpt, fault_gla;
if ( tb_init_done )
{
@@ -2371,11 +2372,31 @@ static void ept_handle_violation(unsigned long qualification, paddr_t gpa)
}
if ( qualification & EPT_GLA_VALID )
+ {
__vmread(GUEST_LINEAR_ADDRESS, &gla);
+
+ if ( qualification & EPT_GLA_FAULT )
+ {
+ fault_in_gpt = 0;
+ fault_gla = 1;
+ }
+ else
+ {
+ fault_in_gpt = 1;
+ fault_gla = 0;
+ }
+ }
else
+ {
gla = ~0ull;
+ fault_in_gpt = 0;
+ fault_gla = 0;
+ }
ret = hvm_hap_nested_page_fault(gpa,
- !!(qualification & EPT_GLA_VALID), gla,
+ !!(qualification & EPT_GLA_VALID),
+ gla,
+ fault_in_gpt,
+ fault_gla,
!!(qualification & EPT_READ_VIOLATION),
!!(qualification & EPT_WRITE_VIOLATION),
!!(qualification & EPT_EXEC_VIOLATION));
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index bca9f0f..132f0d2 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1323,7 +1323,8 @@ void p2m_mem_paging_resume(struct domain *d)
}
}
-bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
+bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
+ bool_t fault_in_gpt, bool_t fault_gla,
bool_t access_r, bool_t access_w, bool_t access_x,
mem_event_request_t **req_ptr)
{
@@ -1405,6 +1406,8 @@ bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
req->offset = gpa & ((1 << PAGE_SHIFT) - 1);
req->gla_valid = gla_valid;
req->gla = gla;
+ req->fault_in_gpt = fault_in_gpt;
+ req->fault_gla = fault_gla;
req->access_r = access_r;
req->access_w = access_w;
req->access_x = access_x;
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 0ebd478..de755b6 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -456,7 +456,10 @@ static inline void hvm_invalidate_regs_fields(struct cpu_user_regs *regs)
}
int hvm_hap_nested_page_fault(paddr_t gpa,
- bool_t gla_valid, unsigned long gla,
+ bool_t gla_valid,
+ unsigned long gla,
+ bool_t fault_in_gpt,
+ bool_t fault_gla,
bool_t access_r,
bool_t access_w,
bool_t access_x);
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 0ddbadb..59803c5 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -597,7 +597,8 @@ void p2m_mem_paging_resume(struct domain *d);
* been promoted with no underlying vcpu pause. If the req_ptr has been populated,
* then the caller must put the event in the ring (once having released get_gfn*
* locks -- caller must also xfree the request. */
-bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
+bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
+ bool_t fault_in_gpt, bool_t fault_gla,
bool_t access_r, bool_t access_w, bool_t access_x,
mem_event_request_t **req_ptr);
/* Resumes the running of the VCPU, restarting the last instruction */
diff --git a/xen/include/public/mem_event.h b/xen/include/public/mem_event.h
index 3831b41..1ba6863 100644
--- a/xen/include/public/mem_event.h
+++ b/xen/include/public/mem_event.h
@@ -62,7 +62,9 @@ typedef struct mem_event_st {
uint16_t access_w:1;
uint16_t access_x:1;
uint16_t gla_valid:1;
- uint16_t available:12;
+ uint16_t fault_in_gpt:1;
+ uint16_t fault_gla:1;
+ uint16_t available:10;
uint16_t reason;
} mem_event_request_t, mem_event_response_t;
--
2.0.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/2] tools/xen-access: Print gla valid/fault information
2014-08-07 19:47 [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information Tamas K Lengyel
@ 2014-08-07 19:47 ` Tamas K Lengyel
2014-08-07 21:39 ` [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information Boris Ostrovsky
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Tamas K Lengyel @ 2014-08-07 19:47 UTC (permalink / raw)
To: xen-devel
Cc: kevin.tian, ian.campbell, stefano.stabellini, jun.nakajima,
eddie.dong, ian.jackson, Aravind.Gopalakrishnan,
suravee.suthikulpanit, Tamas K Lengyel, boris.ostrovsky
Extend the print-out of the memory violations to show gla valid/fault information.
v2: Update to new fields and change printing 1/0 to y/n.
Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
---
tools/tests/xen-access/xen-access.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 090df5f..378b205 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -566,13 +566,16 @@ int main(int argc, char *argv[])
}
printf("PAGE ACCESS: %c%c%c for GFN %"PRIx64" (offset %06"
- PRIx64") gla %016"PRIx64" (vcpu %d)\n",
+ PRIx64") gla %016"PRIx64" (valid: %c) fault in gpt: %c gpa has gla: %c (vcpu %d)\n",
req.access_r ? 'r' : '-',
req.access_w ? 'w' : '-',
req.access_x ? 'x' : '-',
req.gfn,
req.offset,
req.gla,
+ req.gla_valid ? 'y' : 'n',
+ req.fault_in_gpt ? 'y' : 'n',
+ req.fault_gla ? 'y': 'n',
req.vcpu_id);
if ( default_access != after_first_access )
--
2.0.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-07 19:47 [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information Tamas K Lengyel
2014-08-07 19:47 ` [PATCH v2 2/2] tools/xen-access: Print gla valid/fault information Tamas K Lengyel
@ 2014-08-07 21:39 ` Boris Ostrovsky
2014-08-07 21:53 ` Tamas Lengyel
2014-08-07 22:00 ` Andrew Cooper
2014-08-07 22:53 ` Tian, Kevin
3 siblings, 1 reply; 14+ messages in thread
From: Boris Ostrovsky @ 2014-08-07 21:39 UTC (permalink / raw)
To: Tamas K Lengyel, xen-devel
Cc: kevin.tian, ian.campbell, stefano.stabellini, eddie.dong,
ian.jackson, Aravind.Gopalakrishnan, jun.nakajima,
suravee.suthikulpanit
On 08/07/2014 03:47 PM, Tamas K Lengyel wrote:
> On Intel EPT the exit qualification generated by a violation also includes a bit (EPT_GLA_FAULT) which describes the following information:
> Set if the access causing the EPT violation is to a guest-physical address that is the translation of a linear address. Clear if the access causing the EPT violation is to a paging-structure entry as part of a page walk or the update of an accessed or dirty bit.
>
> For more information see Table 27-7 in the Intel SDM.
>
> This patch extends the mem_event system to deliver this extra information, which could be useful for determining the cause of a violation.
>
> v2: Split gla_fault into fault_in_gpt and fault_gla to be more compatible with the AMD implementation.
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
> ---
> xen/arch/x86/hvm/hvm.c | 8 ++++++--
> xen/arch/x86/hvm/svm/svm.c | 2 +-
> xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++++++-
> xen/arch/x86/mm/p2m.c | 5 ++++-
> xen/include/asm-x86/hvm/hvm.h | 5 ++++-
> xen/include/asm-x86/p2m.h | 3 ++-
> xen/include/public/mem_event.h | 4 +++-
> 7 files changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index e834406..d7b5e2b 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned long cr2)
> int hvm_hap_nested_page_fault(paddr_t gpa,
> bool_t gla_valid,
> unsigned long gla,
> + bool_t fault_in_gpt,
> + bool_t fault_gla,
> bool_t access_r,
> bool_t access_w,
> bool_t access_x)
> @@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>
> if ( violation )
> {
> - if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
> - access_w, access_x, &req_ptr) )
> + if ( p2m_mem_access_check(gpa, gla_valid, gla,
> + fault_in_gpt, fault_gla,
> + access_r, access_w, access_x,
> + &req_ptr) )
> {
> fall_through = 1;
> } else {
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 76616ac..9e35e7a 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1403,7 +1403,7 @@ static void svm_do_nested_pgfault(struct vcpu *v,
> p2m_access_t p2ma;
> struct p2m_domain *p2m = NULL;
>
> - ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul,
> + ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul, 0, 0,
Why not pass the actual bits that the HW provides?
-boris
> 1, /* All NPFs count as reads */
> npfec & PFEC_write_access,
> npfec & PFEC_insn_fetch);
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 2caa04a..7ebf289 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2353,6 +2353,7 @@ static void ept_handle_violation(unsigned long qualification, paddr_t gpa)
> p2m_type_t p2mt;
> int ret;
> struct domain *d = current->domain;
> + bool_t fault_in_gpt, fault_gla;
>
> if ( tb_init_done )
> {
> @@ -2371,11 +2372,31 @@ static void ept_handle_violation(unsigned long qualification, paddr_t gpa)
> }
>
> if ( qualification & EPT_GLA_VALID )
> + {
> __vmread(GUEST_LINEAR_ADDRESS, &gla);
> +
> + if ( qualification & EPT_GLA_FAULT )
> + {
> + fault_in_gpt = 0;
> + fault_gla = 1;
> + }
> + else
> + {
> + fault_in_gpt = 1;
> + fault_gla = 0;
> + }
> + }
> else
> + {
> gla = ~0ull;
> + fault_in_gpt = 0;
> + fault_gla = 0;
> + }
> ret = hvm_hap_nested_page_fault(gpa,
> - !!(qualification & EPT_GLA_VALID), gla,
> + !!(qualification & EPT_GLA_VALID),
> + gla,
> + fault_in_gpt,
> + fault_gla,
> !!(qualification & EPT_READ_VIOLATION),
> !!(qualification & EPT_WRITE_VIOLATION),
> !!(qualification & EPT_EXEC_VIOLATION));
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index bca9f0f..132f0d2 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1323,7 +1323,8 @@ void p2m_mem_paging_resume(struct domain *d)
> }
> }
>
> -bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
> +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
> + bool_t fault_in_gpt, bool_t fault_gla,
> bool_t access_r, bool_t access_w, bool_t access_x,
> mem_event_request_t **req_ptr)
> {
> @@ -1405,6 +1406,8 @@ bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
> req->offset = gpa & ((1 << PAGE_SHIFT) - 1);
> req->gla_valid = gla_valid;
> req->gla = gla;
> + req->fault_in_gpt = fault_in_gpt;
> + req->fault_gla = fault_gla;
> req->access_r = access_r;
> req->access_w = access_w;
> req->access_x = access_x;
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 0ebd478..de755b6 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -456,7 +456,10 @@ static inline void hvm_invalidate_regs_fields(struct cpu_user_regs *regs)
> }
>
> int hvm_hap_nested_page_fault(paddr_t gpa,
> - bool_t gla_valid, unsigned long gla,
> + bool_t gla_valid,
> + unsigned long gla,
> + bool_t fault_in_gpt,
> + bool_t fault_gla,
> bool_t access_r,
> bool_t access_w,
> bool_t access_x);
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index 0ddbadb..59803c5 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -597,7 +597,8 @@ void p2m_mem_paging_resume(struct domain *d);
> * been promoted with no underlying vcpu pause. If the req_ptr has been populated,
> * then the caller must put the event in the ring (once having released get_gfn*
> * locks -- caller must also xfree the request. */
> -bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
> +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
> + bool_t fault_in_gpt, bool_t fault_gla,
> bool_t access_r, bool_t access_w, bool_t access_x,
> mem_event_request_t **req_ptr);
> /* Resumes the running of the VCPU, restarting the last instruction */
> diff --git a/xen/include/public/mem_event.h b/xen/include/public/mem_event.h
> index 3831b41..1ba6863 100644
> --- a/xen/include/public/mem_event.h
> +++ b/xen/include/public/mem_event.h
> @@ -62,7 +62,9 @@ typedef struct mem_event_st {
> uint16_t access_w:1;
> uint16_t access_x:1;
> uint16_t gla_valid:1;
> - uint16_t available:12;
> + uint16_t fault_in_gpt:1;
> + uint16_t fault_gla:1;
> + uint16_t available:10;
>
> uint16_t reason;
> } mem_event_request_t, mem_event_response_t;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-07 21:39 ` [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information Boris Ostrovsky
@ 2014-08-07 21:53 ` Tamas Lengyel
2014-08-07 22:58 ` Andrew Cooper
0 siblings, 1 reply; 14+ messages in thread
From: Tamas Lengyel @ 2014-08-07 21:53 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: kevin.tian, Ian Campbell, Stefano Stabellini, Jun Nakajima,
eddie.dong, Ian Jackson, Aravind.Gopalakrishnan,
suravee.suthikulpanit, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 8594 bytes --]
On Thu, Aug 7, 2014 at 11:39 PM, Boris Ostrovsky <boris.ostrovsky@oracle.com
> wrote:
> On 08/07/2014 03:47 PM, Tamas K Lengyel wrote:
>
>> On Intel EPT the exit qualification generated by a violation also
>> includes a bit (EPT_GLA_FAULT) which describes the following information:
>> Set if the access causing the EPT violation is to a guest-physical
>> address that is the translation of a linear address. Clear if the access
>> causing the EPT violation is to a paging-structure entry as part of a page
>> walk or the update of an accessed or dirty bit.
>>
>> For more information see Table 27-7 in the Intel SDM.
>>
>> This patch extends the mem_event system to deliver this extra
>> information, which could be useful for determining the cause of a violation.
>>
>> v2: Split gla_fault into fault_in_gpt and fault_gla to be more compatible
>> with the AMD implementation.
>>
>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
>> ---
>> xen/arch/x86/hvm/hvm.c | 8 ++++++--
>> xen/arch/x86/hvm/svm/svm.c | 2 +-
>> xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++++++-
>> xen/arch/x86/mm/p2m.c | 5 ++++-
>> xen/include/asm-x86/hvm/hvm.h | 5 ++++-
>> xen/include/asm-x86/p2m.h | 3 ++-
>> xen/include/public/mem_event.h | 4 +++-
>> 7 files changed, 42 insertions(+), 8 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> index e834406..d7b5e2b 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned
>> long cr2)
>> int hvm_hap_nested_page_fault(paddr_t gpa,
>> bool_t gla_valid,
>> unsigned long gla,
>> + bool_t fault_in_gpt,
>> + bool_t fault_gla,
>> bool_t access_r,
>> bool_t access_w,
>> bool_t access_x)
>> @@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>> if ( violation )
>> {
>> - if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
>> - access_w, access_x, &req_ptr) )
>> + if ( p2m_mem_access_check(gpa, gla_valid, gla,
>> + fault_in_gpt, fault_gla,
>> + access_r, access_w, access_x,
>> + &req_ptr) )
>> {
>> fall_through = 1;
>> } else {
>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>> index 76616ac..9e35e7a 100644
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -1403,7 +1403,7 @@ static void svm_do_nested_pgfault(struct vcpu *v,
>> p2m_access_t p2ma;
>> struct p2m_domain *p2m = NULL;
>> - ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul,
>> + ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul, 0, 0,
>>
>
>
> Why not pass the actual bits that the HW provides?
>
The actual bits could be passed but it makes no difference at this point
since the AMD side isn't setup to work with mem_event. When it is
integrated, those bits could and should be passed accordingly.
Tamas
>
> -boris
>
>
>
> 1, /* All NPFs count as reads */
>> npfec & PFEC_write_access,
>> npfec & PFEC_insn_fetch);
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index 2caa04a..7ebf289 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -2353,6 +2353,7 @@ static void ept_handle_violation(unsigned long
>> qualification, paddr_t gpa)
>> p2m_type_t p2mt;
>> int ret;
>> struct domain *d = current->domain;
>> + bool_t fault_in_gpt, fault_gla;
>> if ( tb_init_done )
>> {
>> @@ -2371,11 +2372,31 @@ static void ept_handle_violation(unsigned long
>> qualification, paddr_t gpa)
>> }
>> if ( qualification & EPT_GLA_VALID )
>> + {
>> __vmread(GUEST_LINEAR_ADDRESS, &gla);
>> +
>> + if ( qualification & EPT_GLA_FAULT )
>> + {
>> + fault_in_gpt = 0;
>> + fault_gla = 1;
>> + }
>> + else
>> + {
>> + fault_in_gpt = 1;
>> + fault_gla = 0;
>> + }
>> + }
>> else
>> + {
>> gla = ~0ull;
>> + fault_in_gpt = 0;
>> + fault_gla = 0;
>> + }
>> ret = hvm_hap_nested_page_fault(gpa,
>> - !!(qualification & EPT_GLA_VALID),
>> gla,
>> + !!(qualification & EPT_GLA_VALID),
>> + gla,
>> + fault_in_gpt,
>> + fault_gla,
>> !!(qualification &
>> EPT_READ_VIOLATION),
>> !!(qualification &
>> EPT_WRITE_VIOLATION),
>> !!(qualification &
>> EPT_EXEC_VIOLATION));
>> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
>> index bca9f0f..132f0d2 100644
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -1323,7 +1323,8 @@ void p2m_mem_paging_resume(struct domain *d)
>> }
>> }
>> -bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned
>> long gla,
>> +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long
>> gla,
>> + bool_t fault_in_gpt, bool_t fault_gla,
>> bool_t access_r, bool_t access_w, bool_t
>> access_x,
>> mem_event_request_t **req_ptr)
>> {
>> @@ -1405,6 +1406,8 @@ bool_t p2m_mem_access_check(paddr_t gpa, bool_t
>> gla_valid, unsigned long gla,
>> req->offset = gpa & ((1 << PAGE_SHIFT) - 1);
>> req->gla_valid = gla_valid;
>> req->gla = gla;
>> + req->fault_in_gpt = fault_in_gpt;
>> + req->fault_gla = fault_gla;
>> req->access_r = access_r;
>> req->access_w = access_w;
>> req->access_x = access_x;
>> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.
>> h
>> index 0ebd478..de755b6 100644
>> --- a/xen/include/asm-x86/hvm/hvm.h
>> +++ b/xen/include/asm-x86/hvm/hvm.h
>> @@ -456,7 +456,10 @@ static inline void hvm_invalidate_regs_fields(struct
>> cpu_user_regs *regs)
>> }
>> int hvm_hap_nested_page_fault(paddr_t gpa,
>> - bool_t gla_valid, unsigned long gla,
>> + bool_t gla_valid,
>> + unsigned long gla,
>> + bool_t fault_in_gpt,
>> + bool_t fault_gla,
>> bool_t access_r,
>> bool_t access_w,
>> bool_t access_x);
>> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
>> index 0ddbadb..59803c5 100644
>> --- a/xen/include/asm-x86/p2m.h
>> +++ b/xen/include/asm-x86/p2m.h
>> @@ -597,7 +597,8 @@ void p2m_mem_paging_resume(struct domain *d);
>> * been promoted with no underlying vcpu pause. If the req_ptr has been
>> populated,
>> * then the caller must put the event in the ring (once having released
>> get_gfn*
>> * locks -- caller must also xfree the request. */
>> -bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long
>> gla,
>> +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long
>> gla,
>> + bool_t fault_in_gpt, bool_t fault_gla,
>> bool_t access_r, bool_t access_w, bool_t
>> access_x,
>> mem_event_request_t **req_ptr);
>> /* Resumes the running of the VCPU, restarting the last instruction */
>> diff --git a/xen/include/public/mem_event.h b/xen/include/public/mem_
>> event.h
>> index 3831b41..1ba6863 100644
>> --- a/xen/include/public/mem_event.h
>> +++ b/xen/include/public/mem_event.h
>> @@ -62,7 +62,9 @@ typedef struct mem_event_st {
>> uint16_t access_w:1;
>> uint16_t access_x:1;
>> uint16_t gla_valid:1;
>> - uint16_t available:12;
>> + uint16_t fault_in_gpt:1;
>> + uint16_t fault_gla:1;
>> + uint16_t available:10;
>> uint16_t reason;
>> } mem_event_request_t, mem_event_response_t;
>>
>
>
[-- Attachment #1.2: Type: text/html, Size: 10040 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-07 19:47 [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information Tamas K Lengyel
2014-08-07 19:47 ` [PATCH v2 2/2] tools/xen-access: Print gla valid/fault information Tamas K Lengyel
2014-08-07 21:39 ` [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information Boris Ostrovsky
@ 2014-08-07 22:00 ` Andrew Cooper
2014-08-07 22:19 ` Tamas Lengyel
2014-08-07 22:53 ` Tian, Kevin
3 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2014-08-07 22:00 UTC (permalink / raw)
To: Tamas K Lengyel, xen-devel
Cc: kevin.tian, ian.campbell, stefano.stabellini, eddie.dong,
ian.jackson, Aravind.Gopalakrishnan, jun.nakajima,
boris.ostrovsky, suravee.suthikulpanit
On 07/08/2014 20:47, Tamas K Lengyel wrote:
> On Intel EPT the exit qualification generated by a violation also includes a bit (EPT_GLA_FAULT) which describes the following information:
> Set if the access causing the EPT violation is to a guest-physical address that is the translation of a linear address. Clear if the access causing the EPT violation is to a paging-structure entry as part of a page walk or the update of an accessed or dirty bit.
>
> For more information see Table 27-7 in the Intel SDM.
>
> This patch extends the mem_event system to deliver this extra information, which could be useful for determining the cause of a violation.
>
> v2: Split gla_fault into fault_in_gpt and fault_gla to be more compatible with the AMD implementation.
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
> ---
> xen/arch/x86/hvm/hvm.c | 8 ++++++--
> xen/arch/x86/hvm/svm/svm.c | 2 +-
> xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++++++-
> xen/arch/x86/mm/p2m.c | 5 ++++-
> xen/include/asm-x86/hvm/hvm.h | 5 ++++-
> xen/include/asm-x86/p2m.h | 3 ++-
> xen/include/public/mem_event.h | 4 +++-
> 7 files changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index e834406..d7b5e2b 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned long cr2)
> int hvm_hap_nested_page_fault(paddr_t gpa,
> bool_t gla_valid,
> unsigned long gla,
> + bool_t fault_in_gpt,
> + bool_t fault_gla,
> bool_t access_r,
> bool_t access_w,
> bool_t access_x)
> @@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>
> if ( violation )
> {
> - if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
> - access_w, access_x, &req_ptr) )
> + if ( p2m_mem_access_check(gpa, gla_valid, gla,
> + fault_in_gpt, fault_gla,
> + access_r, access_w, access_x,
> + &req_ptr) )
Please fix your new indentation.
> {
> fall_through = 1;
> } else {
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 76616ac..9e35e7a 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1403,7 +1403,7 @@ static void svm_do_nested_pgfault(struct vcpu *v,
> p2m_access_t p2ma;
> struct p2m_domain *p2m = NULL;
>
> - ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul,
> + ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul, 0, 0,
> 1, /* All NPFs count as reads */
> npfec & PFEC_write_access,
> npfec & PFEC_insn_fetch);
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 2caa04a..7ebf289 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2353,6 +2353,7 @@ static void ept_handle_violation(unsigned long qualification, paddr_t gpa)
> p2m_type_t p2mt;
> int ret;
> struct domain *d = current->domain;
> + bool_t fault_in_gpt, fault_gla;
>
> if ( tb_init_done )
> {
> @@ -2371,11 +2372,31 @@ static void ept_handle_violation(unsigned long qualification, paddr_t gpa)
> }
>
> if ( qualification & EPT_GLA_VALID )
> + {
> __vmread(GUEST_LINEAR_ADDRESS, &gla);
> +
> + if ( qualification & EPT_GLA_FAULT )
> + {
> + fault_in_gpt = 0;
> + fault_gla = 1;
> + }
> + else
> + {
> + fault_in_gpt = 1;
> + fault_gla = 0;
> + }
fault_gla = !!(qualification & EPT_GLA_FAULT);
fault_in_gpt = !fault_gla;
is rather shorter.
> + }
> else
> + {
> gla = ~0ull;
> + fault_in_gpt = 0;
> + fault_gla = 0;
These two could arguably be part of the same compound assignment.
> + }
> ret = hvm_hap_nested_page_fault(gpa,
> - !!(qualification & EPT_GLA_VALID), gla,
> + !!(qualification & EPT_GLA_VALID),
> + gla,
> + fault_in_gpt,
> + fault_gla,
These three really should be following each other on the same line.
~Andrew
> !!(qualification & EPT_READ_VIOLATION),
> !!(qualification & EPT_WRITE_VIOLATION),
> !!(qualification & EPT_EXEC_VIOLATION));
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index bca9f0f..132f0d2 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1323,7 +1323,8 @@ void p2m_mem_paging_resume(struct domain *d)
> }
> }
>
> -bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
> +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
> + bool_t fault_in_gpt, bool_t fault_gla,
> bool_t access_r, bool_t access_w, bool_t access_x,
> mem_event_request_t **req_ptr)
> {
> @@ -1405,6 +1406,8 @@ bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
> req->offset = gpa & ((1 << PAGE_SHIFT) - 1);
> req->gla_valid = gla_valid;
> req->gla = gla;
> + req->fault_in_gpt = fault_in_gpt;
> + req->fault_gla = fault_gla;
> req->access_r = access_r;
> req->access_w = access_w;
> req->access_x = access_x;
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 0ebd478..de755b6 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -456,7 +456,10 @@ static inline void hvm_invalidate_regs_fields(struct cpu_user_regs *regs)
> }
>
> int hvm_hap_nested_page_fault(paddr_t gpa,
> - bool_t gla_valid, unsigned long gla,
> + bool_t gla_valid,
> + unsigned long gla,
> + bool_t fault_in_gpt,
> + bool_t fault_gla,
> bool_t access_r,
> bool_t access_w,
> bool_t access_x);
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index 0ddbadb..59803c5 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -597,7 +597,8 @@ void p2m_mem_paging_resume(struct domain *d);
> * been promoted with no underlying vcpu pause. If the req_ptr has been populated,
> * then the caller must put the event in the ring (once having released get_gfn*
> * locks -- caller must also xfree the request. */
> -bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
> +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long gla,
> + bool_t fault_in_gpt, bool_t fault_gla,
> bool_t access_r, bool_t access_w, bool_t access_x,
> mem_event_request_t **req_ptr);
> /* Resumes the running of the VCPU, restarting the last instruction */
> diff --git a/xen/include/public/mem_event.h b/xen/include/public/mem_event.h
> index 3831b41..1ba6863 100644
> --- a/xen/include/public/mem_event.h
> +++ b/xen/include/public/mem_event.h
> @@ -62,7 +62,9 @@ typedef struct mem_event_st {
> uint16_t access_w:1;
> uint16_t access_x:1;
> uint16_t gla_valid:1;
> - uint16_t available:12;
> + uint16_t fault_in_gpt:1;
> + uint16_t fault_gla:1;
> + uint16_t available:10;
>
> uint16_t reason;
> } mem_event_request_t, mem_event_response_t;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-07 22:00 ` Andrew Cooper
@ 2014-08-07 22:19 ` Tamas Lengyel
2014-08-07 22:54 ` Andrew Cooper
0 siblings, 1 reply; 14+ messages in thread
From: Tamas Lengyel @ 2014-08-07 22:19 UTC (permalink / raw)
To: Andrew Cooper
Cc: kevin.tian, Ian Campbell, Stefano Stabellini, Ian Jackson,
eddie.dong, Aravind.Gopalakrishnan, Jun Nakajima, xen-devel,
Boris Ostrovsky, suravee.suthikulpanit
[-- Attachment #1.1: Type: text/plain, Size: 8843 bytes --]
On Fri, Aug 8, 2014 at 12:00 AM, Andrew Cooper <andrew.cooper3@citrix.com>
wrote:
> On 07/08/2014 20:47, Tamas K Lengyel wrote:
> > On Intel EPT the exit qualification generated by a violation also
> includes a bit (EPT_GLA_FAULT) which describes the following information:
> > Set if the access causing the EPT violation is to a guest-physical
> address that is the translation of a linear address. Clear if the access
> causing the EPT violation is to a paging-structure entry as part of a page
> walk or the update of an accessed or dirty bit.
> >
> > For more information see Table 27-7 in the Intel SDM.
> >
> > This patch extends the mem_event system to deliver this extra
> information, which could be useful for determining the cause of a violation.
> >
> > v2: Split gla_fault into fault_in_gpt and fault_gla to be more
> compatible with the AMD implementation.
> >
> > Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
> > ---
> > xen/arch/x86/hvm/hvm.c | 8 ++++++--
> > xen/arch/x86/hvm/svm/svm.c | 2 +-
> > xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++++++-
> > xen/arch/x86/mm/p2m.c | 5 ++++-
> > xen/include/asm-x86/hvm/hvm.h | 5 ++++-
> > xen/include/asm-x86/p2m.h | 3 ++-
> > xen/include/public/mem_event.h | 4 +++-
> > 7 files changed, 42 insertions(+), 8 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > index e834406..d7b5e2b 100644
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned
> long cr2)
> > int hvm_hap_nested_page_fault(paddr_t gpa,
> > bool_t gla_valid,
> > unsigned long gla,
> > + bool_t fault_in_gpt,
> > + bool_t fault_gla,
> > bool_t access_r,
> > bool_t access_w,
> > bool_t access_x)
> > @@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
> >
> > if ( violation )
> > {
> > - if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
> > - access_w, access_x, &req_ptr) )
> > + if ( p2m_mem_access_check(gpa, gla_valid, gla,
> > + fault_in_gpt, fault_gla,
> > + access_r, access_w, access_x,
> > + &req_ptr) )
>
> Please fix your new indentation.
>
I'm just guessing here but is it the preferred indentation to fill each
line to about the same length before starting a new line?
>
>
> {
> > fall_through = 1;
> > } else {
> > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> > index 76616ac..9e35e7a 100644
> > --- a/xen/arch/x86/hvm/svm/svm.c
> > +++ b/xen/arch/x86/hvm/svm/svm.c
> > @@ -1403,7 +1403,7 @@ static void svm_do_nested_pgfault(struct vcpu *v,
> > p2m_access_t p2ma;
> > struct p2m_domain *p2m = NULL;
> >
> > - ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul,
> > + ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul, 0, 0,
> > 1, /* All NPFs count as reads */
> > npfec & PFEC_write_access,
> > npfec & PFEC_insn_fetch);
> > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> > index 2caa04a..7ebf289 100644
> > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > @@ -2353,6 +2353,7 @@ static void ept_handle_violation(unsigned long
> qualification, paddr_t gpa)
> > p2m_type_t p2mt;
> > int ret;
> > struct domain *d = current->domain;
> > + bool_t fault_in_gpt, fault_gla;
> >
> > if ( tb_init_done )
> > {
> > @@ -2371,11 +2372,31 @@ static void ept_handle_violation(unsigned long
> qualification, paddr_t gpa)
> > }
> >
> > if ( qualification & EPT_GLA_VALID )
> > + {
> > __vmread(GUEST_LINEAR_ADDRESS, &gla);
> > +
> > + if ( qualification & EPT_GLA_FAULT )
> > + {
> > + fault_in_gpt = 0;
> > + fault_gla = 1;
> > + }
> > + else
> > + {
> > + fault_in_gpt = 1;
> > + fault_gla = 0;
> > + }
>
> fault_gla = !!(qualification & EPT_GLA_FAULT);
> fault_in_gpt = !fault_gla;
>
> is rather shorter.
>
Ack.
>
> > + }
> > else
> > + {
> > gla = ~0ull;
> > + fault_in_gpt = 0;
> > + fault_gla = 0;
>
> These two could arguably be part of the same compound assignment.
>
>
Ack.
> > + }
> > ret = hvm_hap_nested_page_fault(gpa,
> > - !!(qualification & EPT_GLA_VALID),
> gla,
> > + !!(qualification & EPT_GLA_VALID),
> > + gla,
> > + fault_in_gpt,
> > + fault_gla,
>
> These three really should be following each other on the same line.
>
Ack.
>
> ~Andrew
>
> > !!(qualification &
> EPT_READ_VIOLATION),
> > !!(qualification &
> EPT_WRITE_VIOLATION),
> > !!(qualification &
> EPT_EXEC_VIOLATION));
> > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> > index bca9f0f..132f0d2 100644
> > --- a/xen/arch/x86/mm/p2m.c
> > +++ b/xen/arch/x86/mm/p2m.c
> > @@ -1323,7 +1323,8 @@ void p2m_mem_paging_resume(struct domain *d)
> > }
> > }
> >
> > -bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned
> long gla,
> > +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned
> long gla,
> > + bool_t fault_in_gpt, bool_t fault_gla,
> > bool_t access_r, bool_t access_w, bool_t
> access_x,
> > mem_event_request_t **req_ptr)
> > {
> > @@ -1405,6 +1406,8 @@ bool_t p2m_mem_access_check(paddr_t gpa, bool_t
> gla_valid, unsigned long gla,
> > req->offset = gpa & ((1 << PAGE_SHIFT) - 1);
> > req->gla_valid = gla_valid;
> > req->gla = gla;
> > + req->fault_in_gpt = fault_in_gpt;
> > + req->fault_gla = fault_gla;
> > req->access_r = access_r;
> > req->access_w = access_w;
> > req->access_x = access_x;
> > diff --git a/xen/include/asm-x86/hvm/hvm.h
> b/xen/include/asm-x86/hvm/hvm.h
> > index 0ebd478..de755b6 100644
> > --- a/xen/include/asm-x86/hvm/hvm.h
> > +++ b/xen/include/asm-x86/hvm/hvm.h
> > @@ -456,7 +456,10 @@ static inline void
> hvm_invalidate_regs_fields(struct cpu_user_regs *regs)
> > }
> >
> > int hvm_hap_nested_page_fault(paddr_t gpa,
> > - bool_t gla_valid, unsigned long gla,
> > + bool_t gla_valid,
> > + unsigned long gla,
> > + bool_t fault_in_gpt,
> > + bool_t fault_gla,
> > bool_t access_r,
> > bool_t access_w,
> > bool_t access_x);
> > diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> > index 0ddbadb..59803c5 100644
> > --- a/xen/include/asm-x86/p2m.h
> > +++ b/xen/include/asm-x86/p2m.h
> > @@ -597,7 +597,8 @@ void p2m_mem_paging_resume(struct domain *d);
> > * been promoted with no underlying vcpu pause. If the req_ptr has been
> populated,
> > * then the caller must put the event in the ring (once having released
> get_gfn*
> > * locks -- caller must also xfree the request. */
> > -bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned
> long gla,
> > +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned
> long gla,
> > + bool_t fault_in_gpt, bool_t fault_gla,
> > bool_t access_r, bool_t access_w, bool_t
> access_x,
> > mem_event_request_t **req_ptr);
> > /* Resumes the running of the VCPU, restarting the last instruction */
> > diff --git a/xen/include/public/mem_event.h
> b/xen/include/public/mem_event.h
> > index 3831b41..1ba6863 100644
> > --- a/xen/include/public/mem_event.h
> > +++ b/xen/include/public/mem_event.h
> > @@ -62,7 +62,9 @@ typedef struct mem_event_st {
> > uint16_t access_w:1;
> > uint16_t access_x:1;
> > uint16_t gla_valid:1;
> > - uint16_t available:12;
> > + uint16_t fault_in_gpt:1;
> > + uint16_t fault_gla:1;
> > + uint16_t available:10;
> >
> > uint16_t reason;
> > } mem_event_request_t, mem_event_response_t;
>
>
[-- Attachment #1.2: Type: text/html, Size: 11325 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-07 19:47 [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information Tamas K Lengyel
` (2 preceding siblings ...)
2014-08-07 22:00 ` Andrew Cooper
@ 2014-08-07 22:53 ` Tian, Kevin
3 siblings, 0 replies; 14+ messages in thread
From: Tian, Kevin @ 2014-08-07 22:53 UTC (permalink / raw)
To: Tamas K Lengyel, xen-devel
Cc: ian.campbell, stefano.stabellini, Nakajima, Jun, Dong, Eddie,
ian.jackson, Aravind.Gopalakrishnan, suravee.suthikulpanit,
boris.ostrovsky
> From: Tamas K Lengyel [mailto:tamas.lengyel@zentific.com]
> Sent: Thursday, August 07, 2014 12:47 PM
>
> On Intel EPT the exit qualification generated by a violation also includes a bit
> (EPT_GLA_FAULT) which describes the following information:
> Set if the access causing the EPT violation is to a guest-physical address that is
> the translation of a linear address. Clear if the access causing the EPT violation
> is to a paging-structure entry as part of a page walk or the update of an
> accessed or dirty bit.
>
> For more information see Table 27-7 in the Intel SDM.
>
> This patch extends the mem_event system to deliver this extra information,
> which could be useful for determining the cause of a violation.
>
> v2: Split gla_fault into fault_in_gpt and fault_gla to be more compatible with
> the AMD implementation.
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
basically no problem to me, and I'll ack your next version after you address
coding comments from Andrea.
Thanks
Kevin
> ---
> xen/arch/x86/hvm/hvm.c | 8 ++++++--
> xen/arch/x86/hvm/svm/svm.c | 2 +-
> xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++++++-
> xen/arch/x86/mm/p2m.c | 5 ++++-
> xen/include/asm-x86/hvm/hvm.h | 5 ++++-
> xen/include/asm-x86/p2m.h | 3 ++-
> xen/include/public/mem_event.h | 4 +++-
> 7 files changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index e834406..d7b5e2b 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned
> long cr2)
> int hvm_hap_nested_page_fault(paddr_t gpa,
> bool_t gla_valid,
> unsigned long gla,
> + bool_t fault_in_gpt,
> + bool_t fault_gla,
> bool_t access_r,
> bool_t access_w,
> bool_t access_x)
> @@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>
> if ( violation )
> {
> - if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
> - access_w, access_x,
> &req_ptr) )
> + if ( p2m_mem_access_check(gpa, gla_valid, gla,
> + fault_in_gpt, fault_gla,
> + access_r, access_w,
> access_x,
> + &req_ptr) )
> {
> fall_through = 1;
> } else {
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 76616ac..9e35e7a 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1403,7 +1403,7 @@ static void svm_do_nested_pgfault(struct vcpu *v,
> p2m_access_t p2ma;
> struct p2m_domain *p2m = NULL;
>
> - ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul,
> + ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul, 0, 0,
> 1, /* All NPFs count as reads */
> npfec & PFEC_write_access,
> npfec & PFEC_insn_fetch);
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 2caa04a..7ebf289 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2353,6 +2353,7 @@ static void ept_handle_violation(unsigned long
> qualification, paddr_t gpa)
> p2m_type_t p2mt;
> int ret;
> struct domain *d = current->domain;
> + bool_t fault_in_gpt, fault_gla;
>
> if ( tb_init_done )
> {
> @@ -2371,11 +2372,31 @@ static void ept_handle_violation(unsigned long
> qualification, paddr_t gpa)
> }
>
> if ( qualification & EPT_GLA_VALID )
> + {
> __vmread(GUEST_LINEAR_ADDRESS, &gla);
> +
> + if ( qualification & EPT_GLA_FAULT )
> + {
> + fault_in_gpt = 0;
> + fault_gla = 1;
> + }
> + else
> + {
> + fault_in_gpt = 1;
> + fault_gla = 0;
> + }
> + }
> else
> + {
> gla = ~0ull;
> + fault_in_gpt = 0;
> + fault_gla = 0;
> + }
> ret = hvm_hap_nested_page_fault(gpa,
> - !!(qualification &
> EPT_GLA_VALID), gla,
> + !!(qualification &
> EPT_GLA_VALID),
> + gla,
> + fault_in_gpt,
> + fault_gla,
> !!(qualification &
> EPT_READ_VIOLATION),
> !!(qualification &
> EPT_WRITE_VIOLATION),
> !!(qualification &
> EPT_EXEC_VIOLATION));
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index bca9f0f..132f0d2 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1323,7 +1323,8 @@ void p2m_mem_paging_resume(struct domain *d)
> }
> }
>
> -bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long
> gla,
> +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long
> gla,
> + bool_t fault_in_gpt, bool_t fault_gla,
> bool_t access_r, bool_t access_w, bool_t
> access_x,
> mem_event_request_t **req_ptr)
> {
> @@ -1405,6 +1406,8 @@ bool_t p2m_mem_access_check(paddr_t gpa,
> bool_t gla_valid, unsigned long gla,
> req->offset = gpa & ((1 << PAGE_SHIFT) - 1);
> req->gla_valid = gla_valid;
> req->gla = gla;
> + req->fault_in_gpt = fault_in_gpt;
> + req->fault_gla = fault_gla;
> req->access_r = access_r;
> req->access_w = access_w;
> req->access_x = access_x;
> diff --git a/xen/include/asm-x86/hvm/hvm.h
> b/xen/include/asm-x86/hvm/hvm.h
> index 0ebd478..de755b6 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -456,7 +456,10 @@ static inline void hvm_invalidate_regs_fields(struct
> cpu_user_regs *regs)
> }
>
> int hvm_hap_nested_page_fault(paddr_t gpa,
> - bool_t gla_valid, unsigned long gla,
> + bool_t gla_valid,
> + unsigned long gla,
> + bool_t fault_in_gpt,
> + bool_t fault_gla,
> bool_t access_r,
> bool_t access_w,
> bool_t access_x);
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index 0ddbadb..59803c5 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -597,7 +597,8 @@ void p2m_mem_paging_resume(struct domain *d);
> * been promoted with no underlying vcpu pause. If the req_ptr has been
> populated,
> * then the caller must put the event in the ring (once having released
> get_gfn*
> * locks -- caller must also xfree the request. */
> -bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long
> gla,
> +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long
> gla,
> + bool_t fault_in_gpt, bool_t fault_gla,
> bool_t access_r, bool_t access_w, bool_t
> access_x,
> mem_event_request_t **req_ptr);
> /* Resumes the running of the VCPU, restarting the last instruction */
> diff --git a/xen/include/public/mem_event.h
> b/xen/include/public/mem_event.h
> index 3831b41..1ba6863 100644
> --- a/xen/include/public/mem_event.h
> +++ b/xen/include/public/mem_event.h
> @@ -62,7 +62,9 @@ typedef struct mem_event_st {
> uint16_t access_w:1;
> uint16_t access_x:1;
> uint16_t gla_valid:1;
> - uint16_t available:12;
> + uint16_t fault_in_gpt:1;
> + uint16_t fault_gla:1;
> + uint16_t available:10;
>
> uint16_t reason;
> } mem_event_request_t, mem_event_response_t;
> --
> 2.0.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-07 22:19 ` Tamas Lengyel
@ 2014-08-07 22:54 ` Andrew Cooper
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2014-08-07 22:54 UTC (permalink / raw)
To: Tamas Lengyel
Cc: kevin.tian, Ian Campbell, Stefano Stabellini, Ian Jackson,
eddie.dong, Aravind.Gopalakrishnan, Jun Nakajima, xen-devel,
Boris Ostrovsky, suravee.suthikulpanit
On 07/08/2014 23:19, Tamas Lengyel wrote:
> On Fri, Aug 8, 2014 at 12:00 AM, Andrew Cooper <andrew.cooper3@citrix.com>
> wrote:
>
>> On 07/08/2014 20:47, Tamas K Lengyel wrote:
>>> On Intel EPT the exit qualification generated by a violation also
>> includes a bit (EPT_GLA_FAULT) which describes the following information:
>>> Set if the access causing the EPT violation is to a guest-physical
>> address that is the translation of a linear address. Clear if the access
>> causing the EPT violation is to a paging-structure entry as part of a page
>> walk or the update of an accessed or dirty bit.
>>>
>>> For more information see Table 27-7 in the Intel SDM.
>>>
>>> This patch extends the mem_event system to deliver this extra
>> information, which could be useful for determining the cause of a violation.
>>>
>>> v2: Split gla_fault into fault_in_gpt and fault_gla to be more
>> compatible with the AMD implementation.
>>>
>>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
>>> ---
>>> xen/arch/x86/hvm/hvm.c | 8 ++++++--
>>> xen/arch/x86/hvm/svm/svm.c | 2 +-
>>> xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++++++-
>>> xen/arch/x86/mm/p2m.c | 5 ++++-
>>> xen/include/asm-x86/hvm/hvm.h | 5 ++++-
>>> xen/include/asm-x86/p2m.h | 3 ++-
>>> xen/include/public/mem_event.h | 4 +++-
>>> 7 files changed, 42 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>> index e834406..d7b5e2b 100644
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned
>> long cr2)
>>> int hvm_hap_nested_page_fault(paddr_t gpa,
>>> bool_t gla_valid,
>>> unsigned long gla,
>>> + bool_t fault_in_gpt,
>>> + bool_t fault_gla,
>>> bool_t access_r,
>>> bool_t access_w,
>>> bool_t access_x)
>>> @@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>>>
>>> if ( violation )
>>> {
>>> - if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
>>> - access_w, access_x, &req_ptr) )
>>> + if ( p2m_mem_access_check(gpa, gla_valid, gla,
>>> + fault_in_gpt, fault_gla,
>>> + access_r, access_w, access_x,
>>> + &req_ptr) )
>>
>> Please fix your new indentation.
>>
>
> I'm just guessing here but is it the preferred indentation to fill each
> line to about the same length before starting a new line?
Line up each line of arguments with the g of gpa. The original patch
was two spaces out.
~Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-07 21:53 ` Tamas Lengyel
@ 2014-08-07 22:58 ` Andrew Cooper
2014-08-07 23:03 ` Tamas Lengyel
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2014-08-07 22:58 UTC (permalink / raw)
To: Tamas Lengyel, Boris Ostrovsky
Cc: kevin.tian, Ian Campbell, Stefano Stabellini, eddie.dong,
Ian Jackson, Aravind.Gopalakrishnan, Jun Nakajima, xen-devel,
suravee.suthikulpanit
On 07/08/2014 22:53, Tamas Lengyel wrote:
> On Thu, Aug 7, 2014 at 11:39 PM, Boris Ostrovsky <boris.ostrovsky@oracle.com
>> wrote:
>> On 08/07/2014 03:47 PM, Tamas K Lengyel wrote:
>>
>>> On Intel EPT the exit qualification generated by a violation also
>>> includes a bit (EPT_GLA_FAULT) which describes the following information:
>>> Set if the access causing the EPT violation is to a guest-physical
>>> address that is the translation of a linear address. Clear if the access
>>> causing the EPT violation is to a paging-structure entry as part of a page
>>> walk or the update of an accessed or dirty bit.
>>>
>>> For more information see Table 27-7 in the Intel SDM.
>>>
>>> This patch extends the mem_event system to deliver this extra
>>> information, which could be useful for determining the cause of a violation.
>>>
>>> v2: Split gla_fault into fault_in_gpt and fault_gla to be more compatible
>>> with the AMD implementation.
>>>
>>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
>>> ---
>>> xen/arch/x86/hvm/hvm.c | 8 ++++++--
>>> xen/arch/x86/hvm/svm/svm.c | 2 +-
>>> xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++++++-
>>> xen/arch/x86/mm/p2m.c | 5 ++++-
>>> xen/include/asm-x86/hvm/hvm.h | 5 ++++-
>>> xen/include/asm-x86/p2m.h | 3 ++-
>>> xen/include/public/mem_event.h | 4 +++-
>>> 7 files changed, 42 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>> index e834406..d7b5e2b 100644
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned
>>> long cr2)
>>> int hvm_hap_nested_page_fault(paddr_t gpa,
>>> bool_t gla_valid,
>>> unsigned long gla,
>>> + bool_t fault_in_gpt,
>>> + bool_t fault_gla,
>>> bool_t access_r,
>>> bool_t access_w,
>>> bool_t access_x)
>>> @@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>>> if ( violation )
>>> {
>>> - if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
>>> - access_w, access_x, &req_ptr) )
>>> + if ( p2m_mem_access_check(gpa, gla_valid, gla,
>>> + fault_in_gpt, fault_gla,
>>> + access_r, access_w, access_x,
>>> + &req_ptr) )
>>> {
>>> fall_through = 1;
>>> } else {
>>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>>> index 76616ac..9e35e7a 100644
>>> --- a/xen/arch/x86/hvm/svm/svm.c
>>> +++ b/xen/arch/x86/hvm/svm/svm.c
>>> @@ -1403,7 +1403,7 @@ static void svm_do_nested_pgfault(struct vcpu *v,
>>> p2m_access_t p2ma;
>>> struct p2m_domain *p2m = NULL;
>>> - ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul,
>>> + ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul, 0, 0,
>>>
>>
>> Why not pass the actual bits that the HW provides?
>>
> The actual bits could be passed but it makes no difference at this point
> since the AMD side isn't setup to work with mem_event. When it is
> integrated, those bits could and should be passed accordingly.
>
> Tamas
There is a lot more than mem_event which might want these bits from AMD.
If the bits are easily available at this point, you should fill them in.
~Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-07 22:58 ` Andrew Cooper
@ 2014-08-07 23:03 ` Tamas Lengyel
2014-08-07 23:05 ` Andrew Cooper
2014-08-08 14:22 ` Boris Ostrovsky
0 siblings, 2 replies; 14+ messages in thread
From: Tamas Lengyel @ 2014-08-07 23:03 UTC (permalink / raw)
To: Andrew Cooper
Cc: kevin.tian, Ian Campbell, Stefano Stabellini, Ian Jackson,
eddie.dong, Aravind.Gopalakrishnan, Jun Nakajima, xen-devel,
Boris Ostrovsky, suravee.suthikulpanit
[-- Attachment #1.1: Type: text/plain, Size: 4194 bytes --]
On Fri, Aug 8, 2014 at 12:58 AM, Andrew Cooper <andrew.cooper3@citrix.com>
wrote:
> On 07/08/2014 22:53, Tamas Lengyel wrote:
> > On Thu, Aug 7, 2014 at 11:39 PM, Boris Ostrovsky <
> boris.ostrovsky@oracle.com
> >> wrote:
> >> On 08/07/2014 03:47 PM, Tamas K Lengyel wrote:
> >>
> >>> On Intel EPT the exit qualification generated by a violation also
> >>> includes a bit (EPT_GLA_FAULT) which describes the following
> information:
> >>> Set if the access causing the EPT violation is to a guest-physical
> >>> address that is the translation of a linear address. Clear if the
> access
> >>> causing the EPT violation is to a paging-structure entry as part of a
> page
> >>> walk or the update of an accessed or dirty bit.
> >>>
> >>> For more information see Table 27-7 in the Intel SDM.
> >>>
> >>> This patch extends the mem_event system to deliver this extra
> >>> information, which could be useful for determining the cause of a
> violation.
> >>>
> >>> v2: Split gla_fault into fault_in_gpt and fault_gla to be more
> compatible
> >>> with the AMD implementation.
> >>>
> >>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
> >>> ---
> >>> xen/arch/x86/hvm/hvm.c | 8 ++++++--
> >>> xen/arch/x86/hvm/svm/svm.c | 2 +-
> >>> xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++++++-
> >>> xen/arch/x86/mm/p2m.c | 5 ++++-
> >>> xen/include/asm-x86/hvm/hvm.h | 5 ++++-
> >>> xen/include/asm-x86/p2m.h | 3 ++-
> >>> xen/include/public/mem_event.h | 4 +++-
> >>> 7 files changed, 42 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >>> index e834406..d7b5e2b 100644
> >>> --- a/xen/arch/x86/hvm/hvm.c
> >>> +++ b/xen/arch/x86/hvm/hvm.c
> >>> @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned
> >>> long cr2)
> >>> int hvm_hap_nested_page_fault(paddr_t gpa,
> >>> bool_t gla_valid,
> >>> unsigned long gla,
> >>> + bool_t fault_in_gpt,
> >>> + bool_t fault_gla,
> >>> bool_t access_r,
> >>> bool_t access_w,
> >>> bool_t access_x)
> >>> @@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
> >>> if ( violation )
> >>> {
> >>> - if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
> >>> - access_w, access_x, &req_ptr)
> )
> >>> + if ( p2m_mem_access_check(gpa, gla_valid, gla,
> >>> + fault_in_gpt, fault_gla,
> >>> + access_r, access_w, access_x,
> >>> + &req_ptr) )
> >>> {
> >>> fall_through = 1;
> >>> } else {
> >>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> >>> index 76616ac..9e35e7a 100644
> >>> --- a/xen/arch/x86/hvm/svm/svm.c
> >>> +++ b/xen/arch/x86/hvm/svm/svm.c
> >>> @@ -1403,7 +1403,7 @@ static void svm_do_nested_pgfault(struct vcpu *v,
> >>> p2m_access_t p2ma;
> >>> struct p2m_domain *p2m = NULL;
> >>> - ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul,
> >>> + ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul, 0, 0,
> >>>
> >>
> >> Why not pass the actual bits that the HW provides?
> >>
> > The actual bits could be passed but it makes no difference at this point
> > since the AMD side isn't setup to work with mem_event. When it is
> > integrated, those bits could and should be passed accordingly.
> >
> > Tamas
>
> There is a lot more than mem_event which might want these bits from AMD.
>
> If the bits are easily available at this point, you should fill them in.
>
> ~Andrew
>
I checked and there are no typedefs for these bits in the headers. Also,
the EXITINFO1 passed here is truncated to 32-bits and that would need to be
fixed.. so there are more then one issue that would have to be addressed
for this. I think it would justify a separate patch of its own when it is
actually needed.
Tamas
[-- Attachment #1.2: Type: text/html, Size: 5734 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-07 23:03 ` Tamas Lengyel
@ 2014-08-07 23:05 ` Andrew Cooper
2014-08-08 14:22 ` Boris Ostrovsky
1 sibling, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2014-08-07 23:05 UTC (permalink / raw)
To: Tamas Lengyel
Cc: kevin.tian, Ian Campbell, Stefano Stabellini, Ian Jackson,
eddie.dong, Aravind.Gopalakrishnan, Jun Nakajima, xen-devel,
Boris Ostrovsky, suravee.suthikulpanit
On 08/08/2014 00:03, Tamas Lengyel wrote:
> On Fri, Aug 8, 2014 at 12:58 AM, Andrew Cooper <andrew.cooper3@citrix.com>
> wrote:
>
>> On 07/08/2014 22:53, Tamas Lengyel wrote:
>>> On Thu, Aug 7, 2014 at 11:39 PM, Boris Ostrovsky <
>> boris.ostrovsky@oracle.com
>>>> wrote:
>>>> On 08/07/2014 03:47 PM, Tamas K Lengyel wrote:
>>>>
>>>>> On Intel EPT the exit qualification generated by a violation also
>>>>> includes a bit (EPT_GLA_FAULT) which describes the following
>> information:
>>>>> Set if the access causing the EPT violation is to a guest-physical
>>>>> address that is the translation of a linear address. Clear if the
>> access
>>>>> causing the EPT violation is to a paging-structure entry as part of a
>> page
>>>>> walk or the update of an accessed or dirty bit.
>>>>>
>>>>> For more information see Table 27-7 in the Intel SDM.
>>>>>
>>>>> This patch extends the mem_event system to deliver this extra
>>>>> information, which could be useful for determining the cause of a
>> violation.
>>>>> v2: Split gla_fault into fault_in_gpt and fault_gla to be more
>> compatible
>>>>> with the AMD implementation.
>>>>>
>>>>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
>>>>> ---
>>>>> xen/arch/x86/hvm/hvm.c | 8 ++++++--
>>>>> xen/arch/x86/hvm/svm/svm.c | 2 +-
>>>>> xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++++++-
>>>>> xen/arch/x86/mm/p2m.c | 5 ++++-
>>>>> xen/include/asm-x86/hvm/hvm.h | 5 ++++-
>>>>> xen/include/asm-x86/p2m.h | 3 ++-
>>>>> xen/include/public/mem_event.h | 4 +++-
>>>>> 7 files changed, 42 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>>>> index e834406..d7b5e2b 100644
>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>> @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode, unsigned
>>>>> long cr2)
>>>>> int hvm_hap_nested_page_fault(paddr_t gpa,
>>>>> bool_t gla_valid,
>>>>> unsigned long gla,
>>>>> + bool_t fault_in_gpt,
>>>>> + bool_t fault_gla,
>>>>> bool_t access_r,
>>>>> bool_t access_w,
>>>>> bool_t access_x)
>>>>> @@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
>>>>> if ( violation )
>>>>> {
>>>>> - if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r,
>>>>> - access_w, access_x, &req_ptr)
>> )
>>>>> + if ( p2m_mem_access_check(gpa, gla_valid, gla,
>>>>> + fault_in_gpt, fault_gla,
>>>>> + access_r, access_w, access_x,
>>>>> + &req_ptr) )
>>>>> {
>>>>> fall_through = 1;
>>>>> } else {
>>>>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>>>>> index 76616ac..9e35e7a 100644
>>>>> --- a/xen/arch/x86/hvm/svm/svm.c
>>>>> +++ b/xen/arch/x86/hvm/svm/svm.c
>>>>> @@ -1403,7 +1403,7 @@ static void svm_do_nested_pgfault(struct vcpu *v,
>>>>> p2m_access_t p2ma;
>>>>> struct p2m_domain *p2m = NULL;
>>>>> - ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul,
>>>>> + ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul, 0, 0,
>>>>>
>>>> Why not pass the actual bits that the HW provides?
>>>>
>>> The actual bits could be passed but it makes no difference at this point
>>> since the AMD side isn't setup to work with mem_event. When it is
>>> integrated, those bits could and should be passed accordingly.
>>>
>>> Tamas
>> There is a lot more than mem_event which might want these bits from AMD.
>>
>> If the bits are easily available at this point, you should fill them in.
>>
>> ~Andrew
>>
> I checked and there are no typedefs for these bits in the headers. Also,
> the EXITINFO1 passed here is truncated to 32-bits and that would need to be
> fixed.. so there are more then one issue that would have to be addressed
> for this. I think it would justify a separate patch of its own when it is
> actually needed.
>
> Tamas
>
That seems fair enough (although the final call is up to the AMD
maintainers).
~Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-07 23:03 ` Tamas Lengyel
2014-08-07 23:05 ` Andrew Cooper
@ 2014-08-08 14:22 ` Boris Ostrovsky
2014-08-08 14:58 ` Jan Beulich
2014-08-08 17:47 ` Tamas Lengyel
1 sibling, 2 replies; 14+ messages in thread
From: Boris Ostrovsky @ 2014-08-08 14:22 UTC (permalink / raw)
To: Tamas Lengyel, Andrew Cooper
Cc: kevin.tian, Ian Campbell, Stefano Stabellini, eddie.dong,
Ian Jackson, Aravind.Gopalakrishnan, Jun Nakajima, xen-devel,
suravee.suthikulpanit
[-- Attachment #1.1: Type: text/plain, Size: 5011 bytes --]
On 08/07/2014 07:03 PM, Tamas Lengyel wrote:
>
>
>
> On Fri, Aug 8, 2014 at 12:58 AM, Andrew Cooper
> <andrew.cooper3@citrix.com <mailto:andrew.cooper3@citrix.com>> wrote:
>
> On 07/08/2014 22:53, Tamas Lengyel wrote:
> > On Thu, Aug 7, 2014 at 11:39 PM, Boris Ostrovsky
> <boris.ostrovsky@oracle.com <mailto:boris.ostrovsky@oracle.com>
> >> wrote:
> >> On 08/07/2014 03:47 PM, Tamas K Lengyel wrote:
> >>
> >>> On Intel EPT the exit qualification generated by a violation also
> >>> includes a bit (EPT_GLA_FAULT) which describes the following
> information:
> >>> Set if the access causing the EPT violation is to a guest-physical
> >>> address that is the translation of a linear address. Clear if
> the access
> >>> causing the EPT violation is to a paging-structure entry as
> part of a page
> >>> walk or the update of an accessed or dirty bit.
> >>>
> >>> For more information see Table 27-7 in the Intel SDM.
> >>>
> >>> This patch extends the mem_event system to deliver this extra
> >>> information, which could be useful for determining the cause
> of a violation.
> >>>
> >>> v2: Split gla_fault into fault_in_gpt and fault_gla to be more
> compatible
> >>> with the AMD implementation.
> >>>
> >>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com
> <mailto:tamas.lengyel@zentific.com>>
> >>> ---
> >>> xen/arch/x86/hvm/hvm.c | 8 ++++++--
> >>> xen/arch/x86/hvm/svm/svm.c | 2 +-
> >>> xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++++++-
> >>> xen/arch/x86/mm/p2m.c | 5 ++++-
> >>> xen/include/asm-x86/hvm/hvm.h | 5 ++++-
> >>> xen/include/asm-x86/p2m.h | 3 ++-
> >>> xen/include/public/mem_event.h | 4 +++-
> >>> 7 files changed, 42 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >>> index e834406..d7b5e2b 100644
> >>> --- a/xen/arch/x86/hvm/hvm.c
> >>> +++ b/xen/arch/x86/hvm/hvm.c
> >>> @@ -2725,6 +2725,8 @@ void hvm_inject_page_fault(int errcode,
> unsigned
> >>> long cr2)
> >>> int hvm_hap_nested_page_fault(paddr_t gpa,
> >>> bool_t gla_valid,
> >>> unsigned long gla,
> >>> + bool_t fault_in_gpt,
> >>> + bool_t fault_gla,
> >>> bool_t access_r,
> >>> bool_t access_w,
> >>> bool_t access_x)
> >>> @@ -2832,8 +2834,10 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
> >>> if ( violation )
> >>> {
> >>> - if ( p2m_mem_access_check(gpa, gla_valid, gla,
> access_r,
> >>> - access_w, access_x, &req_ptr) )
> >>> + if ( p2m_mem_access_check(gpa, gla_valid, gla,
> >>> + fault_in_gpt, fault_gla,
> >>> + access_r, access_w, access_x,
> >>> + &req_ptr) )
> >>> {
> >>> fall_through = 1;
> >>> } else {
> >>> diff --git a/xen/arch/x86/hvm/svm/svm.c
> b/xen/arch/x86/hvm/svm/svm.c
> >>> index 76616ac..9e35e7a 100644
> >>> --- a/xen/arch/x86/hvm/svm/svm.c
> >>> +++ b/xen/arch/x86/hvm/svm/svm.c
> >>> @@ -1403,7 +1403,7 @@ static void svm_do_nested_pgfault(struct
> vcpu *v,
> >>> p2m_access_t p2ma;
> >>> struct p2m_domain *p2m = NULL;
> >>> - ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul,
> >>> + ret = hvm_hap_nested_page_fault(gpa, 0, ~0ul, 0, 0,
> >>>
> >>
> >> Why not pass the actual bits that the HW provides?
> >>
> > The actual bits could be passed but it makes no difference at
> this point
> > since the AMD side isn't setup to work with mem_event. When it is
> > integrated, those bits could and should be passed accordingly.
> >
> > Tamas
>
> There is a lot more than mem_event which might want these bits
> from AMD.
>
> If the bits are easily available at this point, you should fill
> them in.
>
>
> ~Andrew
>
>
> I checked and there are no typedefs for these bits in the headers.
> Also, the EXITINFO1 passed here is truncated to 32-bits and that would
> need to be fixed.. so there are more then one issue that would have to
> be addressed for this. I think it would justify a separate patch of
> its own when it is actually needed.
I think these are small enough fixes that can be made part of this series.
Otherwise we are going to add new interface that we know at the time of
commit is not used properly (the fact that the called code does not use
the information is not important IMO, as long as there are no negative
consequences).
(I am also curious why we are truncating exitinfo1 to 32 bits and yet
compiler doesn't warn.)
-boris
[-- Attachment #1.2: Type: text/html, Size: 10669 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-08 14:22 ` Boris Ostrovsky
@ 2014-08-08 14:58 ` Jan Beulich
2014-08-08 17:47 ` Tamas Lengyel
1 sibling, 0 replies; 14+ messages in thread
From: Jan Beulich @ 2014-08-08 14:58 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: kevin.tian, Ian Campbell, Stefano Stabellini, Jun Nakajima,
Andrew Cooper, Ian Jackson, eddie.dong, Aravind.Gopalakrishnan,
suravee.suthikulpanit, Tamas Lengyel, xen-devel
>>> On 08.08.14 at 16:22, <boris.ostrovsky@oracle.com> wrote:
> (I am also curious why we are truncating exitinfo1 to 32 bits and yet
> compiler doesn't warn.)
The compiler by default never warns about value truncations - there'd
be way too many false positives.
Jan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information
2014-08-08 14:22 ` Boris Ostrovsky
2014-08-08 14:58 ` Jan Beulich
@ 2014-08-08 17:47 ` Tamas Lengyel
1 sibling, 0 replies; 14+ messages in thread
From: Tamas Lengyel @ 2014-08-08 17:47 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: kevin.tian, Ian Campbell, Stefano Stabellini, Andrew Cooper,
eddie.dong, Aravind.Gopalakrishnan, Jun Nakajima, xen-devel,
suravee.suthikulpanit, Ian Jackson
[-- Attachment #1.1: Type: text/plain, Size: 713 bytes --]
>
> I checked and there are no typedefs for these bits in the headers. Also,
> the EXITINFO1 passed here is truncated to 32-bits and that would need to be
> fixed.. so there are more then one issue that would have to be addressed
> for this. I think it would justify a separate patch of its own when it is
> actually needed.
>
>
> I think these are small enough fixes that can be made part of this series.
>
> Otherwise we are going to add new interface that we know at the time of
> commit is not used properly (the fact that the called code does not use the
> information is not important IMO, as long as there are no negative
> consequences).
>
>
Sounds good to me, will add them in the next iteration.
Tamas
[-- Attachment #1.2: Type: text/html, Size: 1306 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-08-08 17:47 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-07 19:47 [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information Tamas K Lengyel
2014-08-07 19:47 ` [PATCH v2 2/2] tools/xen-access: Print gla valid/fault information Tamas K Lengyel
2014-08-07 21:39 ` [PATCH v2 1/2] x86/mem_event: Deliver gla fault EPT violation information Boris Ostrovsky
2014-08-07 21:53 ` Tamas Lengyel
2014-08-07 22:58 ` Andrew Cooper
2014-08-07 23:03 ` Tamas Lengyel
2014-08-07 23:05 ` Andrew Cooper
2014-08-08 14:22 ` Boris Ostrovsky
2014-08-08 14:58 ` Jan Beulich
2014-08-08 17:47 ` Tamas Lengyel
2014-08-07 22:00 ` Andrew Cooper
2014-08-07 22:19 ` Tamas Lengyel
2014-08-07 22:54 ` Andrew Cooper
2014-08-07 22:53 ` Tian, Kevin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.