* [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
@ 2018-05-11 11:11 Alexandru Isaila
2018-05-11 11:11 ` [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm Alexandru Isaila
2018-05-18 15:30 ` [PATCH v1 1/2] x86/mm: Add mem access rights to NPT Jan Beulich
0 siblings, 2 replies; 21+ messages in thread
From: Alexandru Isaila @ 2018-05-11 11:11 UTC (permalink / raw)
To: xen-devel
Cc: tamas, rcojocaru, george.dunlap, andrew.cooper3, jbeulich,
Isaila Alexandru
From: Isaila Alexandru <aisaila@bitdefender.com>
This patch adds access rights for the NPT pages. The access rights are
saved in bits 59:56 of pte that are manipulated through p2m_set_access()
and p2m_get_access() functions.
The patch follows the ept logic.
Note: It was tested with xen-access write
Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
---
xen/arch/x86/mm/mem_access.c | 4 +-
xen/arch/x86/mm/p2m-pt.c | 85 ++++++++++++++++++++++++++++++++-------
xen/include/asm-x86/mem_access.h | 2 +-
xen/include/asm-x86/x86_64/page.h | 9 +++++
4 files changed, 84 insertions(+), 16 deletions(-)
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index c0cd017..6fb7809 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -221,7 +221,9 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long gla,
{
req->u.mem_access.flags |= MEM_ACCESS_GLA_VALID;
req->u.mem_access.gla = gla;
-
+ }
+ if ( npfec.gla_valid || cpu_has_svm )
+ {
if ( npfec.kind == npfec_kind_with_gla )
req->u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA;
else if ( npfec.kind == npfec_kind_in_gpt )
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index b8c5d2e..28e6718 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -68,7 +68,8 @@
static unsigned long p2m_type_to_flags(const struct p2m_domain *p2m,
p2m_type_t t,
mfn_t mfn,
- unsigned int level)
+ unsigned int level,
+ p2m_access_t access)
{
unsigned long flags;
/*
@@ -87,23 +88,28 @@ static unsigned long p2m_type_to_flags(const struct p2m_domain *p2m,
case p2m_ram_paged:
case p2m_ram_paging_in:
default:
- return flags | _PAGE_NX_BIT;
+ flags |= _PAGE_NX_BIT;
+ break;
case p2m_grant_map_ro:
- return flags | P2M_BASE_FLAGS | _PAGE_NX_BIT;
+ flags |= P2M_BASE_FLAGS | _PAGE_NX_BIT;
+ break;
case p2m_ioreq_server:
flags |= P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT;
if ( p2m->ioreq.flags & XEN_DMOP_IOREQ_MEM_ACCESS_WRITE )
- return flags & ~_PAGE_RW;
- return flags;
+ flags &= ~_PAGE_RW;
+ break;
case p2m_ram_ro:
case p2m_ram_logdirty:
case p2m_ram_shared:
- return flags | P2M_BASE_FLAGS;
+ flags |= P2M_BASE_FLAGS;
+ break;
case p2m_ram_rw:
- return flags | P2M_BASE_FLAGS | _PAGE_RW;
+ flags |= P2M_BASE_FLAGS | _PAGE_RW;
+ break;
case p2m_grant_map_rw:
case p2m_map_foreign:
- return flags | P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT;
+ flags |= P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT;
+ break;
case p2m_mmio_direct:
if ( !rangeset_contains_singleton(mmio_ro_ranges, mfn_x(mfn)) )
flags |= _PAGE_RW;
@@ -112,8 +118,38 @@ static unsigned long p2m_type_to_flags(const struct p2m_domain *p2m,
flags |= _PAGE_PWT;
ASSERT(!level);
}
- return flags | P2M_BASE_FLAGS | _PAGE_PCD;
+ flags |= P2M_BASE_FLAGS | _PAGE_PCD;
+ break;
}
+
+ switch (access)
+ {
+ case p2m_access_r:
+ case p2m_access_w:
+ flags |= _PAGE_NX_BIT;
+ flags &= ~_PAGE_RW;
+ break;
+ case p2m_access_rw:
+ flags |= _PAGE_NX_BIT;
+ break;
+ case p2m_access_n:
+ case p2m_access_n2rwx:
+ flags |= _PAGE_NX_BIT;
+ flags &= ~_PAGE_RW;
+ break;
+ case p2m_access_rx:
+ case p2m_access_wx:
+ case p2m_access_rx2rw:
+ flags &= ~(_PAGE_NX_BIT | _PAGE_RW);
+ break;
+ case p2m_access_x:
+ flags &= ~_PAGE_RW;
+ break;
+ case p2m_access_rwx:
+ default:
+ break;
+ }
+ return flags;
}
@@ -174,6 +210,17 @@ static void p2m_add_iommu_flags(l1_pgentry_t *p2m_entry,
l1e_add_flags(*p2m_entry, iommu_nlevel_to_flags(nlevel, flags));
}
+void p2m_set_access(intpte_t *entry, p2m_access_t access)
+{
+ *entry = (*entry & ~PAGE_ACCESS_BITFIELD_MASK) |
+ ((access & PAGE_ACCESS_MASK) << PAGE_ACCESS_START);
+}
+
+p2m_access_t p2m_get_access(intpte_t entry)
+{
+ return (p2m_access_t)((entry >> PAGE_ACCESS_START) & PAGE_ACCESS_MASK);
+}
+
/* Returns: 0 for success, -errno for failure */
static int
p2m_next_level(struct p2m_domain *p2m, void **table,
@@ -201,6 +248,7 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW);
p2m_add_iommu_flags(&new_entry, level, IOMMUF_readable|IOMMUF_writable);
+ p2m_set_access(&new_entry.l1, p2m->default_access);
p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1);
}
else if ( flags & _PAGE_PSE )
@@ -249,6 +297,7 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
{
new_entry = l1e_from_pfn(pfn | (i << ((level - 1) * PAGETABLE_ORDER)),
flags);
+ p2m_set_access(&new_entry.l1, p2m->default_access);
p2m->write_p2m_entry(p2m, gfn, l1_entry + i, new_entry, level);
}
@@ -256,6 +305,7 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW);
p2m_add_iommu_flags(&new_entry, level, IOMMUF_readable|IOMMUF_writable);
+ p2m_set_access(&new_entry.l1, p2m->default_access);
p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1);
}
else
@@ -420,8 +470,9 @@ static int do_recalc(struct p2m_domain *p2m, unsigned long gfn)
if ( nt != ot )
{
unsigned long mfn = l1e_get_pfn(e);
+ p2m_access_t a = p2m_get_access(e.l1);
unsigned long flags = p2m_type_to_flags(p2m, nt,
- _mfn(mfn), level);
+ _mfn(mfn), level, a);
if ( level )
{
@@ -569,13 +620,14 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
l3e_content = mfn_valid(mfn) || p2m_allows_invalid_mfn(p2mt)
? p2m_l3e_from_pfn(mfn_x(mfn),
- p2m_type_to_flags(p2m, p2mt, mfn, 2))
+ p2m_type_to_flags(p2m, p2mt, mfn, 2, p2ma))
: l3e_empty();
entry_content.l1 = l3e_content.l3;
if ( entry_content.l1 != 0 )
p2m_add_iommu_flags(&entry_content, 0, iommu_pte_flags);
+ p2m_set_access(&entry_content.l1, p2ma);
p2m->write_p2m_entry(p2m, gfn, p2m_entry, entry_content, 3);
/* NB: paging_write_p2m_entry() handles tlb flushes properly */
}
@@ -608,7 +660,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
if ( mfn_valid(mfn) || p2m_allows_invalid_mfn(p2mt) )
entry_content = p2m_l1e_from_pfn(mfn_x(mfn),
- p2m_type_to_flags(p2m, p2mt, mfn, 0));
+ p2m_type_to_flags(p2m, p2mt, mfn, 0, p2ma));
else
entry_content = l1e_empty();
@@ -630,6 +682,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
p2m->ioreq.entry_count--;
}
+ p2m_set_access(&entry_content.l1, p2ma);
/* level 1 entry */
p2m->write_p2m_entry(p2m, gfn, p2m_entry, entry_content, 1);
/* NB: paging_write_p2m_entry() handles tlb flushes properly */
@@ -661,13 +714,14 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
l2e_content = mfn_valid(mfn) || p2m_allows_invalid_mfn(p2mt)
? p2m_l2e_from_pfn(mfn_x(mfn),
- p2m_type_to_flags(p2m, p2mt, mfn, 1))
+ p2m_type_to_flags(p2m, p2mt, mfn, 1, p2ma))
: l2e_empty();
entry_content.l1 = l2e_content.l2;
if ( entry_content.l1 != 0 )
p2m_add_iommu_flags(&entry_content, 0, iommu_pte_flags);
+ p2m_set_access(&entry_content.l1, p2ma);
p2m->write_p2m_entry(p2m, gfn, p2m_entry, entry_content, 2);
/* NB: paging_write_p2m_entry() handles tlb flushes properly */
}
@@ -750,7 +804,7 @@ p2m_pt_get_entry(struct p2m_domain *p2m, gfn_t gfn_,
* XXX we will return p2m_invalid for unmapped gfns */
*t = p2m_mmio_dm;
/* Not implemented except with EPT */
- *a = p2m_access_rwx;
+ *a = p2m_access_n;
if ( gfn > p2m->max_mapped_pfn )
{
@@ -813,6 +867,7 @@ pod_retry_l3:
l1_table_offset(addr));
*t = p2m_recalc_type(recalc || _needs_recalc(flags),
p2m_flags_to_type(flags), p2m, gfn);
+ *a = p2m_get_access(l3e->l3);
unmap_domain_page(l3e);
ASSERT(mfn_valid(mfn) || !p2m_is_ram(*t));
@@ -852,6 +907,7 @@ pod_retry_l2:
mfn = _mfn(l2e_get_pfn(*l2e) + l1_table_offset(addr));
*t = p2m_recalc_type(recalc || _needs_recalc(flags),
p2m_flags_to_type(flags), p2m, gfn);
+ *a = p2m_get_access(l2e->l2);
unmap_domain_page(l2e);
ASSERT(mfn_valid(mfn) || !p2m_is_ram(*t));
@@ -888,6 +944,7 @@ pod_retry_l1:
}
mfn = l1e_get_mfn(*l1e);
*t = p2m_recalc_type(recalc || _needs_recalc(flags), l1t, p2m, gfn);
+ *a = p2m_get_access(l1e->l1);
unmap_domain_page(l1e);
ASSERT(mfn_valid(mfn) || !p2m_is_ram(*t) || p2m_is_paging(*t));
diff --git a/xen/include/asm-x86/mem_access.h b/xen/include/asm-x86/mem_access.h
index 4043c9f..34f2c07 100644
--- a/xen/include/asm-x86/mem_access.h
+++ b/xen/include/asm-x86/mem_access.h
@@ -46,7 +46,7 @@ bool p2m_mem_access_emulate_check(struct vcpu *v,
/* Sanity check for mem_access hardware support */
static inline bool p2m_mem_access_sanity_check(struct domain *d)
{
- return is_hvm_domain(d) && cpu_has_vmx && hap_enabled(d);
+ return is_hvm_domain(d) && hap_enabled(d);
}
#endif /*__ASM_X86_MEM_ACCESS_H__ */
diff --git a/xen/include/asm-x86/x86_64/page.h b/xen/include/asm-x86/x86_64/page.h
index 05a0334..d5f5418 100644
--- a/xen/include/asm-x86/x86_64/page.h
+++ b/xen/include/asm-x86/x86_64/page.h
@@ -159,6 +159,15 @@ static inline intpte_t put_pte_flags(unsigned int x)
*/
#define _PAGE_GUEST_KERNEL (1U<<12)
+/*
+ * Bits 19:16 of a 24-bit flag mask are used to store p2m_access_t.
+ * This corresponds to bits 59:56 of a pte
+ */
+#define PAGE_ACCESS_START 59
+#define PAGE_ACCESS_LEN 4
+#define PAGE_ACCESS_MASK ((1UL << PAGE_ACCESS_LEN) - 1)
+#define PAGE_ACCESS_BITFIELD_MASK (PAGE_ACCESS_MASK << PAGE_ACCESS_START)
+
#define PAGE_HYPERVISOR_RO (__PAGE_HYPERVISOR_RO | _PAGE_GLOBAL)
#define PAGE_HYPERVISOR_RW (__PAGE_HYPERVISOR_RW | _PAGE_GLOBAL)
#define PAGE_HYPERVISOR_RX (__PAGE_HYPERVISOR_RX | _PAGE_GLOBAL)
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm
2018-05-11 11:11 [PATCH v1 1/2] x86/mm: Add mem access rights to NPT Alexandru Isaila
@ 2018-05-11 11:11 ` Alexandru Isaila
2018-05-14 17:56 ` Tamas K Lengyel
` (3 more replies)
2018-05-18 15:30 ` [PATCH v1 1/2] x86/mm: Add mem access rights to NPT Jan Beulich
1 sibling, 4 replies; 21+ messages in thread
From: Alexandru Isaila @ 2018-05-11 11:11 UTC (permalink / raw)
To: xen-devel
Cc: tamas, rcojocaru, george.dunlap, andrew.cooper3, jbeulich,
Alexandru Isaila
Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
---
xen/include/asm-x86/monitor.h | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index c5a86d1..7ef2aa2 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -83,16 +83,13 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
(1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
(1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
(1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
- (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG));
+ (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
+ (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED));
- if ( cpu_has_vmx )
- {
- capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED);
- /* Since we know this is on VMX, we can just call the hvm func */
- if ( hvm_is_singlestep_supported() )
- capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
- }
+ /* Check if we are on VMX and then we can just call the hvm func */
+ if ( cpu_has_vmx && hvm_is_singlestep_supported() )
+ capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
if ( hvm_funcs.set_descriptor_access_exiting )
capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS);
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm
2018-05-11 11:11 ` [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm Alexandru Isaila
@ 2018-05-14 17:56 ` Tamas K Lengyel
2018-05-18 15:32 ` Jan Beulich
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Tamas K Lengyel @ 2018-05-14 17:56 UTC (permalink / raw)
To: Alexandru Isaila
Cc: George Dunlap, Andrew Cooper, Jan Beulich, Razvan Cojocaru,
Xen-devel
On Fri, May 11, 2018 at 5:11 AM, Alexandru Isaila
<aisaila@bitdefender.com> wrote:
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> xen/include/asm-x86/monitor.h | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index c5a86d1..7ef2aa2 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -83,16 +83,13 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
> (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
> (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
> (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
> - (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG));
> + (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
> + (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED));
>
> - if ( cpu_has_vmx )
> - {
> - capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED);
>
> - /* Since we know this is on VMX, we can just call the hvm func */
> - if ( hvm_is_singlestep_supported() )
> - capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
> - }
> + /* Check if we are on VMX and then we can just call the hvm func */
> + if ( cpu_has_vmx && hvm_is_singlestep_supported() )
> + capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
>
> if ( hvm_funcs.set_descriptor_access_exiting )
> capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS);
> --
> 2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm
2018-05-11 11:11 ` [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm Alexandru Isaila
2018-05-14 17:56 ` Tamas K Lengyel
@ 2018-05-18 15:32 ` Jan Beulich
2018-05-18 16:00 ` Tamas K Lengyel
2018-07-02 10:59 ` Jan Beulich
2018-07-05 13:35 ` Jan Beulich
3 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2018-05-18 15:32 UTC (permalink / raw)
To: aisaila; +Cc: George Dunlap, Andrew Cooper, tamas, Razvan Cojocaru, xen-devel
>>> On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
It would be helpful to know whether this patch depends on patch 1 in any way.
If it doesn't, with Tamas'es ack this could go in independent of the other one.
For conveying such information a cover letter is usually helpful.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm
2018-05-18 15:32 ` Jan Beulich
@ 2018-05-18 16:00 ` Tamas K Lengyel
2018-05-18 18:22 ` Razvan Cojocaru
0 siblings, 1 reply; 21+ messages in thread
From: Tamas K Lengyel @ 2018-05-18 16:00 UTC (permalink / raw)
To: Jan Beulich
Cc: Alexandru Isaila, Andrew Cooper, George Dunlap, Razvan Cojocaru,
Xen-devel
On Fri, May 18, 2018 at 9:32 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>
> It would be helpful to know whether this patch depends on patch 1 in any way.
> If it doesn't, with Tamas'es ack this could go in independent of the other one.
> For conveying such information a cover letter is usually helpful.
The two types of events are independent, you can trigger emulation
without mem_access restrictions, although that is the one mostly used.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm
2018-05-18 16:00 ` Tamas K Lengyel
@ 2018-05-18 18:22 ` Razvan Cojocaru
0 siblings, 0 replies; 21+ messages in thread
From: Razvan Cojocaru @ 2018-05-18 18:22 UTC (permalink / raw)
To: Tamas K Lengyel, Jan Beulich
Cc: Alexandru Isaila, Andrew Cooper, George Dunlap, Xen-devel
On 05/18/2018 07:00 PM, Tamas K Lengyel wrote:
> On Fri, May 18, 2018 at 9:32 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
>>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>>
>> It would be helpful to know whether this patch depends on patch 1 in any way.
>> If it doesn't, with Tamas'es ack this could go in independent of the other one.
>> For conveying such information a cover letter is usually helpful.
>
> The two types of events are independent, you can trigger emulation
> without mem_access restrictions, although that is the one mostly used.
Indeed, it usually only makes sense to emulate an instruction that has
caused a page fault vm_event (something that's accessing a restricted
page), but it is possible to set the emulate flag when replying to a CR
or MSR write event.
We've waited this long to send this patch out because at least our use
cases always only involve emulating as a response to page fault events
(for MSR and CR writes we are only interested in being able to deny the
write) - but functionally there's no reason why this patch needs to wait
for the mem_access one.
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm
2018-05-11 11:11 ` [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm Alexandru Isaila
2018-05-14 17:56 ` Tamas K Lengyel
2018-05-18 15:32 ` Jan Beulich
@ 2018-07-02 10:59 ` Jan Beulich
2018-07-02 11:29 ` Alexandru Stefan ISAILA
2018-07-05 13:35 ` Jan Beulich
3 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2018-07-02 10:59 UTC (permalink / raw)
To: aisaila, tamas; +Cc: George Dunlap, Andrew Cooper, Razvan Cojocaru, xen-devel
>>> On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -83,16 +83,13 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
> (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
> (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
> (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
> - (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG));
> + (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
> + (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED));
>
> - if ( cpu_has_vmx )
> - {
> - capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED);
>
> - /* Since we know this is on VMX, we can just call the hvm func */
> - if ( hvm_is_singlestep_supported() )
> - capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
> - }
> + /* Check if we are on VMX and then we can just call the hvm func */
> + if ( cpu_has_vmx && hvm_is_singlestep_supported() )
> + capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
I was about to apply this when I noticed the seemingly unnecessary
cpu_has_vmx here: hvm_is_singlestep_supported() is precisely the
abstraction to make such extra checking unnecessary. If you agree,
I can drop it while applying.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm
2018-07-02 10:59 ` Jan Beulich
@ 2018-07-02 11:29 ` Alexandru Stefan ISAILA
0 siblings, 0 replies; 21+ messages in thread
From: Alexandru Stefan ISAILA @ 2018-07-02 11:29 UTC (permalink / raw)
To: JBeulich@suse.com, tamas@tklengyel.com
Cc: George.Dunlap@eu.citrix.com, andrew.cooper3@citrix.com,
rcojocaru@bitdefender.com, xen-devel@lists.xen.org
On Lu, 2018-07-02 at 04:59 -0600, Jan Beulich wrote:
> >
> > >
> > > >
> > > > On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
> > --- a/xen/include/asm-x86/monitor.h
> > +++ b/xen/include/asm-x86/monitor.h
> > @@ -83,16 +83,13 @@ static inline uint32_t
> > arch_monitor_get_capabilities(struct domain *d)
> > (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
> > (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
> > (1U <<
> > XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
> > - (1U <<
> > XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG));
> > + (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG)
> > |
> > + (1U <<
> > XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED));
> >
> > - if ( cpu_has_vmx )
> > - {
> > - capabilities |= (1U <<
> > XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED);
> >
> > - /* Since we know this is on VMX, we can just call the hvm
> > func */
> > - if ( hvm_is_singlestep_supported() )
> > - capabilities |= (1U <<
> > XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
> > - }
> > + /* Check if we are on VMX and then we can just call the hvm
> > func */
> > + if ( cpu_has_vmx && hvm_is_singlestep_supported() )
> > + capabilities |= (1U <<
> > XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
> I was about to apply this when I noticed the seemingly unnecessary
> cpu_has_vmx here: hvm_is_singlestep_supported() is precisely the
> abstraction to make such extra checking unnecessary. If you agree,
> I can drop it while applying.
>
Yes I agree with this and it is ok to drop it.
Thanks,
Alex
________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm
2018-05-11 11:11 ` [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm Alexandru Isaila
` (2 preceding siblings ...)
2018-07-02 10:59 ` Jan Beulich
@ 2018-07-05 13:35 ` Jan Beulich
3 siblings, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2018-07-05 13:35 UTC (permalink / raw)
To: aisaila; +Cc: George Dunlap, Andrew Cooper, tamas, Razvan Cojocaru, xen-devel
>>> On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
> ---
> xen/include/asm-x86/monitor.h | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index c5a86d1..7ef2aa2 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -83,16 +83,13 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
> (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
> (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
> (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
> - (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG));
> + (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
> + (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED));
>
> - if ( cpu_has_vmx )
> - {
> - capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED);
>
> - /* Since we know this is on VMX, we can just call the hvm func */
> - if ( hvm_is_singlestep_supported() )
> - capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
After applying I've noticed the double blank lines this change leaves.
I've corrected this for you, but please pay attention yourself in the
future.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-05-11 11:11 [PATCH v1 1/2] x86/mm: Add mem access rights to NPT Alexandru Isaila
2018-05-11 11:11 ` [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm Alexandru Isaila
@ 2018-05-18 15:30 ` Jan Beulich
2018-05-18 18:30 ` Razvan Cojocaru
1 sibling, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2018-05-18 15:30 UTC (permalink / raw)
To: aisaila; +Cc: George Dunlap, Andrew Cooper, tamas, Razvan Cojocaru, xen-devel
>>> On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
> This patch adds access rights for the NPT pages. The access rights are
> saved in bits 59:56 of pte that are manipulated through p2m_set_access()
> and p2m_get_access() functions.
You don't give any rationale for the choice of bits. Right now p2m-pt.c still
assumes that CPU and IOMMU page tables might be shared, despite
amd_iommu_init() unconditionally turning this functionality off. As long as the
option for that mode hasn't been removed from p2m-pt.c, I think bits used
by the IOMMU (here: bit 59) should not be used for software purposes. The
alternative therefore is for you to supply a prereq patch purging the sharing
functionality from p2m-pt.c and preferably also from the AMD IOMMU code.
That's of course only an option if we don't foresee any means by which this
mode may become usable again.
> --- a/xen/include/asm-x86/x86_64/page.h
> +++ b/xen/include/asm-x86/x86_64/page.h
> @@ -159,6 +159,15 @@ static inline intpte_t put_pte_flags(unsigned int x)
> */
> #define _PAGE_GUEST_KERNEL (1U<<12)
>
> +/*
> + * Bits 19:16 of a 24-bit flag mask are used to store p2m_access_t.
> + * This corresponds to bits 59:56 of a pte
> + */
> +#define PAGE_ACCESS_START 59
> +#define PAGE_ACCESS_LEN 4
> +#define PAGE_ACCESS_MASK ((1UL << PAGE_ACCESS_LEN) - 1)
> +#define PAGE_ACCESS_BITFIELD_MASK (PAGE_ACCESS_MASK << PAGE_ACCESS_START)
Afaics you don't need these outside of p2m-pt.c, in which case I would think
it's better to confine their visibility to that file. That'll avoid the possibly
misleading comment about bits 19:16 here (you never convert the flags to
that form).
Additionally I'd prefer if you defined just a single constant instead of 4 of
them, and if you then used MASK_EXTR() / MASK_INSR() as appropriate.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-05-18 15:30 ` [PATCH v1 1/2] x86/mm: Add mem access rights to NPT Jan Beulich
@ 2018-05-18 18:30 ` Razvan Cojocaru
2018-05-22 9:49 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: Razvan Cojocaru @ 2018-05-18 18:30 UTC (permalink / raw)
To: Jan Beulich, aisaila; +Cc: George Dunlap, Andrew Cooper, tamas, xen-devel
On 05/18/2018 06:30 PM, Jan Beulich wrote:
>>>> On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
>> This patch adds access rights for the NPT pages. The access rights are
>> saved in bits 59:56 of pte that are manipulated through p2m_set_access()
>> and p2m_get_access() functions.
>
> You don't give any rationale for the choice of bits. Right now p2m-pt.c still
> assumes that CPU and IOMMU page tables might be shared, despite
> amd_iommu_init() unconditionally turning this functionality off. As long as the
> option for that mode hasn't been removed from p2m-pt.c, I think bits used
> by the IOMMU (here: bit 59) should not be used for software purposes. The
> alternative therefore is for you to supply a prereq patch purging the sharing
> functionality from p2m-pt.c and preferably also from the AMD IOMMU code.
> That's of course only an option if we don't foresee any means by which this
> mode may become usable again.
The choice of bits was our interpretation of Andrew's reply here:
https://lists.xenproject.org/archives/html/xen-devel/2018-05/msg00573.html
Have we misread it?
We've also thought about putting the information in a new field of
struct page_info. Would that perhaps be preferable?
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-05-18 18:30 ` Razvan Cojocaru
@ 2018-05-22 9:49 ` Jan Beulich
2018-05-22 13:35 ` Alexandru Stefan ISAILA
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2018-05-22 9:49 UTC (permalink / raw)
To: aisaila, Razvan Cojocaru; +Cc: George Dunlap, Andrew Cooper, tamas, xen-devel
>>> On 18.05.18 at 20:30, <rcojocaru@bitdefender.com> wrote:
> On 05/18/2018 06:30 PM, Jan Beulich wrote:
>>>>> On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
>>> This patch adds access rights for the NPT pages. The access rights are
>>> saved in bits 59:56 of pte that are manipulated through p2m_set_access()
>>> and p2m_get_access() functions.
>>
>> You don't give any rationale for the choice of bits. Right now p2m-pt.c still
>> assumes that CPU and IOMMU page tables might be shared, despite
>> amd_iommu_init() unconditionally turning this functionality off. As long as the
>> option for that mode hasn't been removed from p2m-pt.c, I think bits used
>> by the IOMMU (here: bit 59) should not be used for software purposes. The
>> alternative therefore is for you to supply a prereq patch purging the sharing
>> functionality from p2m-pt.c and preferably also from the AMD IOMMU code.
>> That's of course only an option if we don't foresee any means by which this
>> mode may become usable again.
>
> The choice of bits was our interpretation of Andrew's reply here:
>
> https://lists.xenproject.org/archives/html/xen-devel/2018-05/msg00573.html
>
> Have we misread it?
I don't think you have, but what Andrew has described was only the CPU side
of considerations to make. Plus of course the patch description should explain
whatever choice you make.
> We've also thought about putting the information in a new field of
> struct page_info. Would that perhaps be preferable?
I don't view this as a page property, but a mapping property. As such
it can't validly go into struct page_info.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-05-22 9:49 ` Jan Beulich
@ 2018-05-22 13:35 ` Alexandru Stefan ISAILA
2018-05-22 13:44 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: Alexandru Stefan ISAILA @ 2018-05-22 13:35 UTC (permalink / raw)
To: rcojocaru@bitdefender.com, JBeulich@suse.com
Cc: George.Dunlap@eu.citrix.com, andrew.cooper3@citrix.com,
tamas@tklengyel.com, xen-devel@lists.xen.org
On Ma, 2018-05-22 at 03:49 -0600, Jan Beulich wrote:
> >
> > >
> > > >
> > > > On 18.05.18 at 20:30, <rcojocaru@bitdefender.com> wrote:
> > On 05/18/2018 06:30 PM, Jan Beulich wrote:
> > >
> > > >
> > > > >
> > > > > >
> > > > > > On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
> > > > This patch adds access rights for the NPT pages. The access
> > > > rights are
> > > > saved in bits 59:56 of pte that are manipulated through
> > > > p2m_set_access()
> > > > and p2m_get_access() functions.
> > > You don't give any rationale for the choice of bits. Right now
> > > p2m-pt.c still
> > > assumes that CPU and IOMMU page tables might be shared, despite
> > > amd_iommu_init() unconditionally turning this functionality off.
> > > As long as the
> > > option for that mode hasn't been removed from p2m-pt.c, I think
> > > bits used
> > > by the IOMMU (here: bit 59) should not be used for software
> > > purposes. The
> > > alternative therefore is for you to supply a prereq patch purging
> > > the sharing
> > > functionality from p2m-pt.c and preferably also from the AMD
> > > IOMMU code.
> > > That's of course only an option if we don't foresee any means by
> > > which this
> > > mode may become usable again.
> > The choice of bits was our interpretation of Andrew's reply here:
> >
> > https://lists.xenproject.org/archives/html/xen-devel/2018-05/msg005
> > 73.html
> >
> > Have we misread it?
> I don't think you have, but what Andrew has described was only the
> CPU side
> of considerations to make. Plus of course the patch description
> should explain
> whatever choice you make.
>
> >
> > We've also thought about putting the information in a new field of
> > struct page_info. Would that perhaps be preferable?
> I don't view this as a page property, but a mapping property. As such
> it can't validly go into struct page_info.
I will add the information in the patch description. Can you tell us
what structure is best to use for the access rights?
Thanks,
Alex
________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-05-22 13:35 ` Alexandru Stefan ISAILA
@ 2018-05-22 13:44 ` Jan Beulich
2018-05-30 9:04 ` Alexandru Stefan ISAILA
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2018-05-22 13:44 UTC (permalink / raw)
To: aisaila, Razvan Cojocaru; +Cc: George Dunlap, Andrew Cooper, tamas, xen-devel
>>> On 22.05.18 at 15:35, <aisaila@bitdefender.com> wrote:
> On Ma, 2018-05-22 at 03:49 -0600, Jan Beulich wrote:
>> > > > On 18.05.18 at 20:30, <rcojocaru@bitdefender.com> wrote:
>> > On 05/18/2018 06:30 PM, Jan Beulich wrote:
>> > > > > > On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
>> > > > This patch adds access rights for the NPT pages. The access
>> > > > rights are
>> > > > saved in bits 59:56 of pte that are manipulated through
>> > > > p2m_set_access()
>> > > > and p2m_get_access() functions.
>> > > You don't give any rationale for the choice of bits. Right now
>> > > p2m-pt.c still
>> > > assumes that CPU and IOMMU page tables might be shared, despite
>> > > amd_iommu_init() unconditionally turning this functionality off.
>> > > As long as the
>> > > option for that mode hasn't been removed from p2m-pt.c, I think
>> > > bits used
>> > > by the IOMMU (here: bit 59) should not be used for software
>> > > purposes. The
>> > > alternative therefore is for you to supply a prereq patch purging
>> > > the sharing
>> > > functionality from p2m-pt.c and preferably also from the AMD
>> > > IOMMU code.
>> > > That's of course only an option if we don't foresee any means by
>> > > which this
>> > > mode may become usable again.
>> > The choice of bits was our interpretation of Andrew's reply here:
>> >
>> > https://lists.xenproject.org/archives/html/xen-devel/2018-05/msg005
>> > 73.html
>> >
>> > Have we misread it?
>> I don't think you have, but what Andrew has described was only the
>> CPU side
>> of considerations to make. Plus of course the patch description
>> should explain
>> whatever choice you make.
>>
>> >
>> > We've also thought about putting the information in a new field of
>> > struct page_info. Would that perhaps be preferable?
>> I don't view this as a page property, but a mapping property. As such
>> it can't validly go into struct page_info.
>
> I will add the information in the patch description. Can you tell us
> what structure is best to use for the access rights?
I may not correctly understand the question: I think everybody agrees
on the bits to go into an _unused_ portion of the p2m entry.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-05-22 13:44 ` Jan Beulich
@ 2018-05-30 9:04 ` Alexandru Stefan ISAILA
2018-05-30 9:52 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: Alexandru Stefan ISAILA @ 2018-05-30 9:04 UTC (permalink / raw)
To: rcojocaru@bitdefender.com, JBeulich@suse.com
Cc: George.Dunlap@eu.citrix.com, andrew.cooper3@citrix.com,
tamas@tklengyel.com, xen-devel@lists.xen.org
On Ma, 2018-05-22 at 07:44 -0600, Jan Beulich wrote:
> >
> > >
> > > >
> > > > On 22.05.18 at 15:35, <aisaila@bitdefender.com> wrote:
> > On Ma, 2018-05-22 at 03:49 -0600, Jan Beulich wrote:
> > >
> > > >
> > > > >
> > > > > >
> > > > > > On 18.05.18 at 20:30, <rcojocaru@bitdefender.com> wrote:
> > > > On 05/18/2018 06:30 PM, Jan Beulich wrote:
> > > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
> > > > > > This patch adds access rights for the NPT pages. The access
> > > > > > rights are
> > > > > > saved in bits 59:56 of pte that are manipulated through
> > > > > > p2m_set_access()
> > > > > > and p2m_get_access() functions.
> > > > > You don't give any rationale for the choice of bits. Right
> > > > > now
> > > > > p2m-pt.c still
> > > > > assumes that CPU and IOMMU page tables might be shared,
> > > > > despite
> > > > > amd_iommu_init() unconditionally turning this functionality
> > > > > off.
> > > > > As long as the
> > > > > option for that mode hasn't been removed from p2m-pt.c, I
> > > > > think
> > > > > bits used
> > > > > by the IOMMU (here: bit 59) should not be used for software
> > > > > purposes. The
> > > > > alternative therefore is for you to supply a prereq patch
> > > > > purging
> > > > > the sharing
> > > > > functionality from p2m-pt.c and preferably also from the AMD
> > > > > IOMMU code.
> > > > > That's of course only an option if we don't foresee any means
> > > > > by
> > > > > which this
> > > > > mode may become usable again.
> > > > The choice of bits was our interpretation of Andrew's reply
> > > > here:
> > > >
> > > > https://lists.xenproject.org/archives/html/xen-devel/2018-05/ms
> > > > g005
> > > > 73.html
> > > >
> > > > Have we misread it?
> > > I don't think you have, but what Andrew has described was only
> > > the
> > > CPU side
> > > of considerations to make. Plus of course the patch description
> > > should explain
> > > whatever choice you make.
> > >
> > > >
> > > >
> > > > We've also thought about putting the information in a new field
> > > > of
> > > > struct page_info. Would that perhaps be preferable?
> > > I don't view this as a page property, but a mapping property. As
> > > such
> > > it can't validly go into struct page_info.
> > I will add the information in the patch description. Can you tell
> > us
> > what structure is best to use for the access rights?
> I may not correctly understand the question: I think everybody agrees
> on the bits to go into an _unused_ portion of the p2m entry.
Sorry for the misunderstanding, I wanted to clarify if the 59:56 bits
are fully ok to be used or if not then where should I use 4 bits to
store the mem access info?
Any thoughts on this matter are appreciated.
Thanks,
Alex
________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-05-30 9:04 ` Alexandru Stefan ISAILA
@ 2018-05-30 9:52 ` Jan Beulich
2018-05-30 11:20 ` Alexandru Stefan ISAILA
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2018-05-30 9:52 UTC (permalink / raw)
To: aisaila, Razvan Cojocaru; +Cc: George Dunlap, Andrew Cooper, tamas, xen-devel
>>> On 30.05.18 at 11:04, <aisaila@bitdefender.com> wrote:
> Sorry for the misunderstanding, I wanted to clarify if the 59:56 bits
> are fully ok to be used or if not then where should I use 4 bits to
> store the mem access info?
I thought I had sufficiently explained this - you have two options:
1) Make sure (via some prereq patch(es)) bit 59 has no other use, and
then use 59:56.
2) Use another range that's provably having no other use, e.g.
58:55.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-05-30 9:52 ` Jan Beulich
@ 2018-05-30 11:20 ` Alexandru Stefan ISAILA
2018-05-30 11:29 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: Alexandru Stefan ISAILA @ 2018-05-30 11:20 UTC (permalink / raw)
To: rcojocaru@bitdefender.com, JBeulich@suse.com
Cc: George.Dunlap@eu.citrix.com, andrew.cooper3@citrix.com,
tamas@tklengyel.com, xen-devel@lists.xen.org
On Mi, 2018-05-30 at 03:52 -0600, Jan Beulich wrote:
> >
> > >
> > > >
> > > > On 30.05.18 at 11:04, <aisaila@bitdefender.com> wrote:
> > Sorry for the misunderstanding, I wanted to clarify if the 59:56
> > bits
> > are fully ok to be used or if not then where should I use 4 bits to
> > store the mem access info?
> I thought I had sufficiently explained this - you have two options:
> 1) Make sure (via some prereq patch(es)) bit 59 has no other use, and
> then use 59:56.
> 2) Use another range that's provably having no other use, e.g.
> 58:55.
I've checked and bits 40:52 are defined in asm/page.h for page flags.
I've tried bits 53:56 and there where some problems with the guest not
starting or the image freezing, bits 62 and 63 are not free so 59:56 is
the only space to be used for this purpose and is seems to function
correctly. Is there any test I should run?
Thanks,
Alex
________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-05-30 11:20 ` Alexandru Stefan ISAILA
@ 2018-05-30 11:29 ` Jan Beulich
2018-05-30 13:56 ` Andrew Cooper
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2018-05-30 11:29 UTC (permalink / raw)
To: aisaila, Razvan Cojocaru; +Cc: George Dunlap, Andrew Cooper, tamas, xen-devel
>>> On 30.05.18 at 13:20, <aisaila@bitdefender.com> wrote:
> On Mi, 2018-05-30 at 03:52 -0600, Jan Beulich wrote:
>> >
>> > >
>> > > >
>> > > > On 30.05.18 at 11:04, <aisaila@bitdefender.com> wrote:
>> > Sorry for the misunderstanding, I wanted to clarify if the 59:56
>> > bits
>> > are fully ok to be used or if not then where should I use 4 bits to
>> > store the mem access info?
>> I thought I had sufficiently explained this - you have two options:
>> 1) Make sure (via some prereq patch(es)) bit 59 has no other use, and
>> then use 59:56.
>> 2) Use another range that's provably having no other use, e.g.
>> 58:55.
> I've checked and bits 40:52 are defined in asm/page.h for page flags.
40:52? Hardly.
> I've tried bits 53:56 and there where some problems with the guest not
> starting or the image freezing,
Well, you'll have to explain this (perhaps just to yourself).
> bits 62 and 63 are not free so 59:56 is
> the only space to be used for this purpose and is seems to function
> correctly.
Well - as said before, bit 59 is not available for use without some
prereq work.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-05-30 11:29 ` Jan Beulich
@ 2018-05-30 13:56 ` Andrew Cooper
2018-06-05 14:45 ` Alexandru Stefan ISAILA
0 siblings, 1 reply; 21+ messages in thread
From: Andrew Cooper @ 2018-05-30 13:56 UTC (permalink / raw)
To: Jan Beulich, aisaila, Razvan Cojocaru; +Cc: George Dunlap, tamas, xen-devel
On 30/05/18 12:29, Jan Beulich wrote:
>>>> On 30.05.18 at 13:20, <aisaila@bitdefender.com> wrote:
>> On Mi, 2018-05-30 at 03:52 -0600, Jan Beulich wrote:
>>>>>> On 30.05.18 at 11:04, <aisaila@bitdefender.com> wrote:
>>>> Sorry for the misunderstanding, I wanted to clarify if the 59:56
>>>> bits
>>>> are fully ok to be used or if not then where should I use 4 bits to
>>>> store the mem access info?
>>> I thought I had sufficiently explained this - you have two options:
>>> 1) Make sure (via some prereq patch(es)) bit 59 has no other use, and
>>> then use 59:56.
>>> 2) Use another range that's provably having no other use, e.g.
>>> 58:55.
>> I've checked and bits 40:52 are defined in asm/page.h for page flags.
> 40:52? Hardly.
>
>> I've tried bits 53:56 and there where some problems with the guest not
>> starting or the image freezing,
> Well, you'll have to explain this (perhaps just to yourself).
>
>> bits 62 and 63 are not free so 59:56 is
>> the only space to be used for this purpose and is seems to function
>> correctly.
> Well - as said before, bit 59 is not available for use without some
> prereq work.
There are no software available bits in the top of an AMD IOMMU PTE.
Bits 59:62 are defined, while bits 52:58 are strictly reserved and fault
if used.
I'm also not convinced of the safety of our current uses of bits 9:11
which are software available in the regular pagetables, but have
specific meaning in the IOMMU entries.
If the code IOMMU code disables page sharing, then lets go one small
step further and prohibit its use entirely. There is no point trying to
maintain compatibility for an option which isn't used, especially if it
gets in the way of improvements like this in the SVM code.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-05-30 13:56 ` Andrew Cooper
@ 2018-06-05 14:45 ` Alexandru Stefan ISAILA
2018-06-05 15:38 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: Alexandru Stefan ISAILA @ 2018-06-05 14:45 UTC (permalink / raw)
To: rcojocaru@bitdefender.com, JBeulich@suse.com,
andrew.cooper3@citrix.com
Cc: George.Dunlap@eu.citrix.com, tamas@tklengyel.com,
xen-devel@lists.xen.org
On Mi, 2018-05-30 at 14:56 +0100, Andrew Cooper wrote:
> On 30/05/18 12:29, Jan Beulich wrote:
> >
> > >
> > > >
> > > > >
> > > > > On 30.05.18 at 13:20, <aisaila@bitdefender.com> wrote:
> > > On Mi, 2018-05-30 at 03:52 -0600, Jan Beulich wrote:
> > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > On 30.05.18 at 11:04, <aisaila@bitdefender.com> wrote:
> > > > > Sorry for the misunderstanding, I wanted to clarify if the
> > > > > 59:56
> > > > > bits
> > > > > are fully ok to be used or if not then where should I use 4
> > > > > bits to
> > > > > store the mem access info?
> > > > I thought I had sufficiently explained this - you have two
> > > > options:
> > > > 1) Make sure (via some prereq patch(es)) bit 59 has no other
> > > > use, and
> > > > then use 59:56.
> > > > 2) Use another range that's provably having no other use, e.g.
> > > > 58:55.
> > > I've checked and bits 40:52 are defined in asm/page.h for page
> > > flags.
> > 40:52? Hardly.
> >
> > >
> > > I've tried bits 53:56 and there where some problems with the
> > > guest not
> > > starting or the image freezing,
> > Well, you'll have to explain this (perhaps just to yourself).
> >
> > >
> > > bits 62 and 63 are not free so 59:56 is
> > > the only space to be used for this purpose and is seems to
> > > function
> > > correctly.
> > Well - as said before, bit 59 is not available for use without some
> > prereq work.
> There are no software available bits in the top of an AMD IOMMU PTE.
> Bits 59:62 are defined, while bits 52:58 are strictly reserved and
> fault
> if used.
>
> I'm also not convinced of the safety of our current uses of bits 9:11
> which are software available in the regular pagetables, but have
> specific meaning in the IOMMU entries.
>
> If the code IOMMU code disables page sharing, then lets go one small
> step further and prohibit its use entirely. There is no point trying
> to
> maintain compatibility for an option which isn't used, especially if
> it
> gets in the way of improvements like this in the SVM code.
>
Another idea is to save the mem access info in a radix tree like on the
ARM side and we can store the radix tree root in the p2m_domain.
I think that this is the fastest and cleanest way to solve the free
bits problem.
Is this a suitable way to go?
Thanks,
Alex
________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
2018-06-05 14:45 ` Alexandru Stefan ISAILA
@ 2018-06-05 15:38 ` Jan Beulich
0 siblings, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2018-06-05 15:38 UTC (permalink / raw)
To: aisaila; +Cc: George Dunlap, Andrew Cooper, tamas, Razvan Cojocaru, xen-devel
>>> On 05.06.18 at 16:45, <aisaila@bitdefender.com> wrote:
> On Mi, 2018-05-30 at 14:56 +0100, Andrew Cooper wrote:
>> On 30/05/18 12:29, Jan Beulich wrote:
>> >
>> > >
>> > > >
>> > > > >
>> > > > > On 30.05.18 at 13:20, <aisaila@bitdefender.com> wrote:
>> > > On Mi, 2018-05-30 at 03:52 -0600, Jan Beulich wrote:
>> > > >
>> > > > >
>> > > > > >
>> > > > > > >
>> > > > > > > On 30.05.18 at 11:04, <aisaila@bitdefender.com> wrote:
>> > > > > Sorry for the misunderstanding, I wanted to clarify if the
>> > > > > 59:56
>> > > > > bits
>> > > > > are fully ok to be used or if not then where should I use 4
>> > > > > bits to
>> > > > > store the mem access info?
>> > > > I thought I had sufficiently explained this - you have two
>> > > > options:
>> > > > 1) Make sure (via some prereq patch(es)) bit 59 has no other
>> > > > use, and
>> > > > then use 59:56.
>> > > > 2) Use another range that's provably having no other use, e.g.
>> > > > 58:55.
>> > > I've checked and bits 40:52 are defined in asm/page.h for page
>> > > flags.
>> > 40:52? Hardly.
>> >
>> > >
>> > > I've tried bits 53:56 and there where some problems with the
>> > > guest not
>> > > starting or the image freezing,
>> > Well, you'll have to explain this (perhaps just to yourself).
>> >
>> > >
>> > > bits 62 and 63 are not free so 59:56 is
>> > > the only space to be used for this purpose and is seems to
>> > > function
>> > > correctly.
>> > Well - as said before, bit 59 is not available for use without some
>> > prereq work.
>> There are no software available bits in the top of an AMD IOMMU PTE.
>> Bits 59:62 are defined, while bits 52:58 are strictly reserved and
>> fault
>> if used.
>>
>> I'm also not convinced of the safety of our current uses of bits 9:11
>> which are software available in the regular pagetables, but have
>> specific meaning in the IOMMU entries.
>>
>> If the code IOMMU code disables page sharing, then lets go one small
>> step further and prohibit its use entirely. There is no point trying
>> to
>> maintain compatibility for an option which isn't used, especially if
>> it
>> gets in the way of improvements like this in the SVM code.
>>
> Another idea is to save the mem access info in a radix tree like on the
> ARM side and we can store the radix tree root in the p2m_domain.
>
> I think that this is the fastest and cleanest way to solve the free
> bits problem.
But it adds extra (memory, lookup time) overhead.
> Is this a suitable way to go?
If no other option exists - perhaps. But that's more a question to be
answered by George.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2018-07-05 13:35 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-11 11:11 [PATCH v1 1/2] x86/mm: Add mem access rights to NPT Alexandru Isaila
2018-05-11 11:11 ` [PATCH v1 2/2] hvm/svm: Enable EMUL_UNIMPLEMENTED events on svm Alexandru Isaila
2018-05-14 17:56 ` Tamas K Lengyel
2018-05-18 15:32 ` Jan Beulich
2018-05-18 16:00 ` Tamas K Lengyel
2018-05-18 18:22 ` Razvan Cojocaru
2018-07-02 10:59 ` Jan Beulich
2018-07-02 11:29 ` Alexandru Stefan ISAILA
2018-07-05 13:35 ` Jan Beulich
2018-05-18 15:30 ` [PATCH v1 1/2] x86/mm: Add mem access rights to NPT Jan Beulich
2018-05-18 18:30 ` Razvan Cojocaru
2018-05-22 9:49 ` Jan Beulich
2018-05-22 13:35 ` Alexandru Stefan ISAILA
2018-05-22 13:44 ` Jan Beulich
2018-05-30 9:04 ` Alexandru Stefan ISAILA
2018-05-30 9:52 ` Jan Beulich
2018-05-30 11:20 ` Alexandru Stefan ISAILA
2018-05-30 11:29 ` Jan Beulich
2018-05-30 13:56 ` Andrew Cooper
2018-06-05 14:45 ` Alexandru Stefan ISAILA
2018-06-05 15:38 ` Jan Beulich
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.