* [PATCH 12/13] Nested Virtualization: vram
@ 2010-09-01 15:17 Christoph Egger
2010-09-08 15:07 ` Tim Deegan
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Egger @ 2010-09-01 15:17 UTC (permalink / raw)
To: xen-devel@lists.xensource.com; +Cc: Dong, Eddie, Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 322 bytes --]
Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
--
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
[-- Attachment #2: xen_nh12_vram.diff --]
[-- Type: text/x-diff, Size: 9063 bytes --]
# HG changeset patch
# User cegger
# Date 1283345891 -7200
Move dirty_vram from struct hvm_domain to struct p2m_domain
diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/arch/x86/mm/hap/hap.c
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -58,7 +58,8 @@
static int hap_enable_vram_tracking(struct domain *d)
{
int i;
- struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
+ struct p2m_domain *p2m = p2m_get_hostp2m(d);
+ struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
if ( !dirty_vram )
return -EINVAL;
@@ -70,7 +71,7 @@ static int hap_enable_vram_tracking(stru
/* set l1e entries of P2M table to be read-only. */
for (i = dirty_vram->begin_pfn; i < dirty_vram->end_pfn; i++)
- p2m_change_type(p2m_get_hostp2m(d), i, p2m_ram_rw, p2m_ram_logdirty);
+ p2m_change_type(p2m, i, p2m_ram_rw, p2m_ram_logdirty);
flush_tlb_mask(&d->domain_dirty_cpumask);
return 0;
@@ -79,7 +80,8 @@ static int hap_enable_vram_tracking(stru
static int hap_disable_vram_tracking(struct domain *d)
{
int i;
- struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
+ struct p2m_domain *p2m = p2m_get_hostp2m(d);
+ struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
if ( !dirty_vram )
return -EINVAL;
@@ -90,7 +92,7 @@ static int hap_disable_vram_tracking(str
/* set l1e entries of P2M table with normal mode */
for (i = dirty_vram->begin_pfn; i < dirty_vram->end_pfn; i++)
- p2m_change_type(p2m_get_hostp2m(d), i, p2m_ram_logdirty, p2m_ram_rw);
+ p2m_change_type(p2m, i, p2m_ram_logdirty, p2m_ram_rw);
flush_tlb_mask(&d->domain_dirty_cpumask);
return 0;
@@ -99,14 +101,15 @@ static int hap_disable_vram_tracking(str
static void hap_clean_vram_tracking(struct domain *d)
{
int i;
- struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
+ struct p2m_domain *p2m = p2m_get_hostp2m(d);
+ struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
if ( !dirty_vram )
return;
/* set l1e entries of P2M table to be read-only. */
for (i = dirty_vram->begin_pfn; i < dirty_vram->end_pfn; i++)
- p2m_change_type(p2m_get_hostp2m(d), i, p2m_ram_rw, p2m_ram_logdirty);
+ p2m_change_type(p2m, i, p2m_ram_rw, p2m_ram_logdirty);
flush_tlb_mask(&d->domain_dirty_cpumask);
}
@@ -124,7 +127,8 @@ int hap_track_dirty_vram(struct domain *
XEN_GUEST_HANDLE_64(uint8) dirty_bitmap)
{
long rc = 0;
- struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
+ struct p2m_domain *p2m = p2m_get_hostp2m(d);
+ struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
if ( nr )
{
@@ -149,7 +153,7 @@ int hap_track_dirty_vram(struct domain *
dirty_vram->begin_pfn = begin_pfn;
dirty_vram->end_pfn = begin_pfn + nr;
- d->arch.hvm_domain.dirty_vram = dirty_vram;
+ p2m->dirty_vram = dirty_vram;
hap_vram_tracking_init(d);
rc = paging_log_dirty_enable(d);
if (rc != 0)
@@ -171,7 +175,7 @@ int hap_track_dirty_vram(struct domain *
if ( paging_mode_log_dirty(d) && dirty_vram ) {
rc = paging_log_dirty_disable(d);
xfree(dirty_vram);
- dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
+ dirty_vram = p2m->dirty_vram = NULL;
} else
rc = 0;
}
@@ -182,7 +186,7 @@ param_fail:
if ( dirty_vram )
{
xfree(dirty_vram);
- dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
+ dirty_vram = p2m->dirty_vram = NULL;
}
return rc;
}
@@ -228,12 +232,13 @@ static void hap_clean_dirty_bitmap(struc
void hap_logdirty_init(struct domain *d)
{
- struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
+ struct p2m_domain *p2m = p2m_get_hostp2m(d);
+ struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
if ( paging_mode_log_dirty(d) && dirty_vram )
{
paging_log_dirty_disable(d);
xfree(dirty_vram);
- dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
+ dirty_vram = p2m->dirty_vram = NULL;
}
/* Reinitialize logdirty mechanism */
diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/arch/x86/mm/shadow/common.c
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -3211,11 +3211,11 @@ void shadow_teardown(struct domain *d)
* calls now that we've torn down the bitmap */
d->arch.paging.mode &= ~PG_log_dirty;
- if (d->arch.hvm_domain.dirty_vram) {
- xfree(d->arch.hvm_domain.dirty_vram->sl1ma);
- xfree(d->arch.hvm_domain.dirty_vram->dirty_bitmap);
- xfree(d->arch.hvm_domain.dirty_vram);
- d->arch.hvm_domain.dirty_vram = NULL;
+ if (p2m->dirty_vram) {
+ xfree(p2m->dirty_vram->sl1ma);
+ xfree(p2m->dirty_vram->dirty_bitmap);
+ xfree(p2m->dirty_vram);
+ p2m->dirty_vram = NULL;
}
shadow_unlock(d);
@@ -3559,8 +3559,8 @@ int shadow_track_dirty_vram(struct domai
int flush_tlb = 0;
unsigned long i;
p2m_type_t t;
- struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
+ struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
if (end_pfn < begin_pfn
|| begin_pfn > p2m->max_mapped_pfn
@@ -3574,11 +3574,12 @@ int shadow_track_dirty_vram(struct domai
|| end_pfn != dirty_vram->end_pfn )) )
{
/* Different tracking, tear the previous down. */
- gdprintk(XENLOG_INFO, "stopping tracking VRAM %lx - %lx\n", dirty_vram->begin_pfn, dirty_vram->end_pfn);
+ gdprintk(XENLOG_INFO, "stopping tracking VRAM %lx - %lx\n",
+ dirty_vram->begin_pfn, dirty_vram->end_pfn);
xfree(dirty_vram->sl1ma);
xfree(dirty_vram->dirty_bitmap);
xfree(dirty_vram);
- dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
+ dirty_vram = p2m->dirty_vram = NULL;
}
if ( !nr )
@@ -3602,7 +3603,7 @@ int shadow_track_dirty_vram(struct domai
goto out;
dirty_vram->begin_pfn = begin_pfn;
dirty_vram->end_pfn = end_pfn;
- d->arch.hvm_domain.dirty_vram = dirty_vram;
+ p2m->dirty_vram = dirty_vram;
if ( (dirty_vram->sl1ma = xmalloc_array(paddr_t, nr)) == NULL )
goto out_dirty_vram;
@@ -3735,7 +3736,7 @@ out_sl1ma:
xfree(dirty_vram->sl1ma);
out_dirty_vram:
xfree(dirty_vram);
- dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
+ dirty_vram = p2m->dirty_vram = NULL;
out:
shadow_unlock(d);
diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/arch/x86/mm/shadow/multi.c
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -515,7 +515,7 @@ _sh_propagate(struct vcpu *v,
guest_l1e_t guest_entry = { guest_intpte };
shadow_l1e_t *sp = shadow_entry_ptr;
struct domain *d = v->domain;
- struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
+ struct sh_dirty_vram *dirty_vram = p2m_get_hostp2m(d)->dirty_vram;
gfn_t target_gfn = guest_l1e_get_gfn(guest_entry);
u32 pass_thru_flags;
u32 gflags, sflags;
@@ -1105,7 +1105,7 @@ static inline void shadow_vram_get_l1e(s
mfn_t mfn = shadow_l1e_get_mfn(new_sl1e);
int flags = shadow_l1e_get_flags(new_sl1e);
unsigned long gfn;
- struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
+ struct sh_dirty_vram *dirty_vram = p2m_get_hostp2m(d)->dirty_vram;
if ( !dirty_vram /* tracking disabled? */
|| !(flags & _PAGE_RW) /* read-only mapping? */
@@ -1136,7 +1136,7 @@ static inline void shadow_vram_put_l1e(s
mfn_t mfn = shadow_l1e_get_mfn(old_sl1e);
int flags = shadow_l1e_get_flags(old_sl1e);
unsigned long gfn;
- struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
+ struct sh_dirty_vram *dirty_vram = p2m_get_hostp2m(d)->dirty_vram;
if ( !dirty_vram /* tracking disabled? */
|| !(flags & _PAGE_RW) /* read-only mapping? */
diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/include/asm-x86/hvm/domain.h
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -69,9 +69,6 @@ struct hvm_domain {
/* Memory ranges with pinned cache attributes. */
struct list_head pinned_cacheattr_ranges;
- /* VRAM dirty support. */
- struct sh_dirty_vram *dirty_vram;
-
/* If one of vcpus of this domain is in no_fill_mode or
* mtrr/pat between vcpus is not the same, set is_in_uc_mode
*/
diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -172,6 +172,9 @@ struct p2m_domain {
/* Shadow translated domain: p2m mapping */
pagetable_t phys_table;
+ /* VRAM dirty support. */
+ struct sh_dirty_vram *dirty_vram;
+
struct domain *domain; /* back pointer to domain */
/* Pages used to construct the p2m */
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 12/13] Nested Virtualization: vram
2010-09-01 15:17 [PATCH 12/13] Nested Virtualization: vram Christoph Egger
@ 2010-09-08 15:07 ` Tim Deegan
2010-09-08 15:47 ` Christoph Egger
0 siblings, 1 reply; 10+ messages in thread
From: Tim Deegan @ 2010-09-08 15:07 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel@lists.xensource.com, Dong, Eddie
Hi,
Is this needed as part of the nested-HVM series or just an independent
interface change?
Tim.
Content-Description: xen_nh12_vram.diff
> # HG changeset patch
> # User cegger
> # Date 1283345891 -7200
> Move dirty_vram from struct hvm_domain to struct p2m_domain
>
> diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/arch/x86/mm/hap/hap.c
> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -58,7 +58,8 @@
> static int hap_enable_vram_tracking(struct domain *d)
> {
> int i;
> - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> + struct p2m_domain *p2m = p2m_get_hostp2m(d);
> + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
>
> if ( !dirty_vram )
> return -EINVAL;
> @@ -70,7 +71,7 @@ static int hap_enable_vram_tracking(stru
>
> /* set l1e entries of P2M table to be read-only. */
> for (i = dirty_vram->begin_pfn; i < dirty_vram->end_pfn; i++)
> - p2m_change_type(p2m_get_hostp2m(d), i, p2m_ram_rw, p2m_ram_logdirty);
> + p2m_change_type(p2m, i, p2m_ram_rw, p2m_ram_logdirty);
>
> flush_tlb_mask(&d->domain_dirty_cpumask);
> return 0;
> @@ -79,7 +80,8 @@ static int hap_enable_vram_tracking(stru
> static int hap_disable_vram_tracking(struct domain *d)
> {
> int i;
> - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> + struct p2m_domain *p2m = p2m_get_hostp2m(d);
> + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
>
> if ( !dirty_vram )
> return -EINVAL;
> @@ -90,7 +92,7 @@ static int hap_disable_vram_tracking(str
>
> /* set l1e entries of P2M table with normal mode */
> for (i = dirty_vram->begin_pfn; i < dirty_vram->end_pfn; i++)
> - p2m_change_type(p2m_get_hostp2m(d), i, p2m_ram_logdirty, p2m_ram_rw);
> + p2m_change_type(p2m, i, p2m_ram_logdirty, p2m_ram_rw);
>
> flush_tlb_mask(&d->domain_dirty_cpumask);
> return 0;
> @@ -99,14 +101,15 @@ static int hap_disable_vram_tracking(str
> static void hap_clean_vram_tracking(struct domain *d)
> {
> int i;
> - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> + struct p2m_domain *p2m = p2m_get_hostp2m(d);
> + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
>
> if ( !dirty_vram )
> return;
>
> /* set l1e entries of P2M table to be read-only. */
> for (i = dirty_vram->begin_pfn; i < dirty_vram->end_pfn; i++)
> - p2m_change_type(p2m_get_hostp2m(d), i, p2m_ram_rw, p2m_ram_logdirty);
> + p2m_change_type(p2m, i, p2m_ram_rw, p2m_ram_logdirty);
>
> flush_tlb_mask(&d->domain_dirty_cpumask);
> }
> @@ -124,7 +127,8 @@ int hap_track_dirty_vram(struct domain *
> XEN_GUEST_HANDLE_64(uint8) dirty_bitmap)
> {
> long rc = 0;
> - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> + struct p2m_domain *p2m = p2m_get_hostp2m(d);
> + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
>
> if ( nr )
> {
> @@ -149,7 +153,7 @@ int hap_track_dirty_vram(struct domain *
>
> dirty_vram->begin_pfn = begin_pfn;
> dirty_vram->end_pfn = begin_pfn + nr;
> - d->arch.hvm_domain.dirty_vram = dirty_vram;
> + p2m->dirty_vram = dirty_vram;
> hap_vram_tracking_init(d);
> rc = paging_log_dirty_enable(d);
> if (rc != 0)
> @@ -171,7 +175,7 @@ int hap_track_dirty_vram(struct domain *
> if ( paging_mode_log_dirty(d) && dirty_vram ) {
> rc = paging_log_dirty_disable(d);
> xfree(dirty_vram);
> - dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
> + dirty_vram = p2m->dirty_vram = NULL;
> } else
> rc = 0;
> }
> @@ -182,7 +186,7 @@ param_fail:
> if ( dirty_vram )
> {
> xfree(dirty_vram);
> - dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
> + dirty_vram = p2m->dirty_vram = NULL;
> }
> return rc;
> }
> @@ -228,12 +232,13 @@ static void hap_clean_dirty_bitmap(struc
>
> void hap_logdirty_init(struct domain *d)
> {
> - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> + struct p2m_domain *p2m = p2m_get_hostp2m(d);
> + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
> if ( paging_mode_log_dirty(d) && dirty_vram )
> {
> paging_log_dirty_disable(d);
> xfree(dirty_vram);
> - dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
> + dirty_vram = p2m->dirty_vram = NULL;
> }
>
> /* Reinitialize logdirty mechanism */
> diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/arch/x86/mm/shadow/common.c
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -3211,11 +3211,11 @@ void shadow_teardown(struct domain *d)
> * calls now that we've torn down the bitmap */
> d->arch.paging.mode &= ~PG_log_dirty;
>
> - if (d->arch.hvm_domain.dirty_vram) {
> - xfree(d->arch.hvm_domain.dirty_vram->sl1ma);
> - xfree(d->arch.hvm_domain.dirty_vram->dirty_bitmap);
> - xfree(d->arch.hvm_domain.dirty_vram);
> - d->arch.hvm_domain.dirty_vram = NULL;
> + if (p2m->dirty_vram) {
> + xfree(p2m->dirty_vram->sl1ma);
> + xfree(p2m->dirty_vram->dirty_bitmap);
> + xfree(p2m->dirty_vram);
> + p2m->dirty_vram = NULL;
> }
>
> shadow_unlock(d);
> @@ -3559,8 +3559,8 @@ int shadow_track_dirty_vram(struct domai
> int flush_tlb = 0;
> unsigned long i;
> p2m_type_t t;
> - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> struct p2m_domain *p2m = p2m_get_hostp2m(d);
> + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
>
> if (end_pfn < begin_pfn
> || begin_pfn > p2m->max_mapped_pfn
> @@ -3574,11 +3574,12 @@ int shadow_track_dirty_vram(struct domai
> || end_pfn != dirty_vram->end_pfn )) )
> {
> /* Different tracking, tear the previous down. */
> - gdprintk(XENLOG_INFO, "stopping tracking VRAM %lx - %lx\n", dirty_vram->begin_pfn, dirty_vram->end_pfn);
> + gdprintk(XENLOG_INFO, "stopping tracking VRAM %lx - %lx\n",
> + dirty_vram->begin_pfn, dirty_vram->end_pfn);
> xfree(dirty_vram->sl1ma);
> xfree(dirty_vram->dirty_bitmap);
> xfree(dirty_vram);
> - dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
> + dirty_vram = p2m->dirty_vram = NULL;
> }
>
> if ( !nr )
> @@ -3602,7 +3603,7 @@ int shadow_track_dirty_vram(struct domai
> goto out;
> dirty_vram->begin_pfn = begin_pfn;
> dirty_vram->end_pfn = end_pfn;
> - d->arch.hvm_domain.dirty_vram = dirty_vram;
> + p2m->dirty_vram = dirty_vram;
>
> if ( (dirty_vram->sl1ma = xmalloc_array(paddr_t, nr)) == NULL )
> goto out_dirty_vram;
> @@ -3735,7 +3736,7 @@ out_sl1ma:
> xfree(dirty_vram->sl1ma);
> out_dirty_vram:
> xfree(dirty_vram);
> - dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
> + dirty_vram = p2m->dirty_vram = NULL;
>
> out:
> shadow_unlock(d);
> diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/arch/x86/mm/shadow/multi.c
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -515,7 +515,7 @@ _sh_propagate(struct vcpu *v,
> guest_l1e_t guest_entry = { guest_intpte };
> shadow_l1e_t *sp = shadow_entry_ptr;
> struct domain *d = v->domain;
> - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> + struct sh_dirty_vram *dirty_vram = p2m_get_hostp2m(d)->dirty_vram;
> gfn_t target_gfn = guest_l1e_get_gfn(guest_entry);
> u32 pass_thru_flags;
> u32 gflags, sflags;
> @@ -1105,7 +1105,7 @@ static inline void shadow_vram_get_l1e(s
> mfn_t mfn = shadow_l1e_get_mfn(new_sl1e);
> int flags = shadow_l1e_get_flags(new_sl1e);
> unsigned long gfn;
> - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> + struct sh_dirty_vram *dirty_vram = p2m_get_hostp2m(d)->dirty_vram;
>
> if ( !dirty_vram /* tracking disabled? */
> || !(flags & _PAGE_RW) /* read-only mapping? */
> @@ -1136,7 +1136,7 @@ static inline void shadow_vram_put_l1e(s
> mfn_t mfn = shadow_l1e_get_mfn(old_sl1e);
> int flags = shadow_l1e_get_flags(old_sl1e);
> unsigned long gfn;
> - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> + struct sh_dirty_vram *dirty_vram = p2m_get_hostp2m(d)->dirty_vram;
>
> if ( !dirty_vram /* tracking disabled? */
> || !(flags & _PAGE_RW) /* read-only mapping? */
> diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/include/asm-x86/hvm/domain.h
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -69,9 +69,6 @@ struct hvm_domain {
> /* Memory ranges with pinned cache attributes. */
> struct list_head pinned_cacheattr_ranges;
>
> - /* VRAM dirty support. */
> - struct sh_dirty_vram *dirty_vram;
> -
> /* If one of vcpus of this domain is in no_fill_mode or
> * mtrr/pat between vcpus is not the same, set is_in_uc_mode
> */
> diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/include/asm-x86/p2m.h
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -172,6 +172,9 @@ struct p2m_domain {
> /* Shadow translated domain: p2m mapping */
> pagetable_t phys_table;
>
> + /* VRAM dirty support. */
> + struct sh_dirty_vram *dirty_vram;
> +
> struct domain *domain; /* back pointer to domain */
>
> /* Pages used to construct the p2m */
--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: Re: [PATCH 12/13] Nested Virtualization: vram
2010-09-08 15:07 ` Tim Deegan
@ 2010-09-08 15:47 ` Christoph Egger
2010-09-13 13:16 ` Tim Deegan
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Egger @ 2010-09-08 15:47 UTC (permalink / raw)
To: xen-devel; +Cc: Dong, Eddie, Tim Deegan
On Wednesday 08 September 2010 17:07:35 Tim Deegan wrote:
> Hi,
>
> Is this needed as part of the nested-HVM series or just an independent
> interface change?
This is an independent interface change.
It is not needed to make nested-HVM work in general but it is part of
a larger change to make log dirty code nested virtualization aware.
Christoph
> Tim.
>
> Content-Description: xen_nh12_vram.diff
>
> > # HG changeset patch
> > # User cegger
> > # Date 1283345891 -7200
> > Move dirty_vram from struct hvm_domain to struct p2m_domain
> >
> > diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/arch/x86/mm/hap/hap.c
> > --- a/xen/arch/x86/mm/hap/hap.c
> > +++ b/xen/arch/x86/mm/hap/hap.c
> > @@ -58,7 +58,8 @@
> > static int hap_enable_vram_tracking(struct domain *d)
> > {
> > int i;
> > - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> > + struct p2m_domain *p2m = p2m_get_hostp2m(d);
> > + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
> >
> > if ( !dirty_vram )
> > return -EINVAL;
> > @@ -70,7 +71,7 @@ static int hap_enable_vram_tracking(stru
> >
> > /* set l1e entries of P2M table to be read-only. */
> > for (i = dirty_vram->begin_pfn; i < dirty_vram->end_pfn; i++)
> > - p2m_change_type(p2m_get_hostp2m(d), i, p2m_ram_rw,
> > p2m_ram_logdirty);
> > + p2m_change_type(p2m, i, p2m_ram_rw,
> > p2m_ram_logdirty);
> >
> > flush_tlb_mask(&d->domain_dirty_cpumask);
> > return 0;
> > @@ -79,7 +80,8 @@ static int hap_enable_vram_tracking(stru
> > static int hap_disable_vram_tracking(struct domain *d)
> > {
> > int i;
> > - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> > + struct p2m_domain *p2m = p2m_get_hostp2m(d);
> > + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
> >
> > if ( !dirty_vram )
> > return -EINVAL;
> > @@ -90,7 +92,7 @@ static int hap_disable_vram_tracking(str
> >
> > /* set l1e entries of P2M table with normal mode */
> > for (i = dirty_vram->begin_pfn; i < dirty_vram->end_pfn; i++)
> > - p2m_change_type(p2m_get_hostp2m(d), i, p2m_ram_logdirty,
> > p2m_ram_rw);
> > + p2m_change_type(p2m, i, p2m_ram_logdirty,
> > p2m_ram_rw);
> >
> > flush_tlb_mask(&d->domain_dirty_cpumask);
> > return 0;
> > @@ -99,14 +101,15 @@ static int hap_disable_vram_tracking(str
> > static void hap_clean_vram_tracking(struct domain *d)
> > {
> > int i;
> > - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> > + struct p2m_domain *p2m = p2m_get_hostp2m(d);
> > + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
> >
> > if ( !dirty_vram )
> > return;
> >
> > /* set l1e entries of P2M table to be read-only. */
> > for (i = dirty_vram->begin_pfn; i < dirty_vram->end_pfn; i++)
> > - p2m_change_type(p2m_get_hostp2m(d), i, p2m_ram_rw,
> > p2m_ram_logdirty); + p2m_change_type(p2m, i, p2m_ram_rw,
> > p2m_ram_logdirty);
> >
> > flush_tlb_mask(&d->domain_dirty_cpumask);
> > }
> > @@ -124,7 +127,8 @@ int hap_track_dirty_vram(struct domain *
> > XEN_GUEST_HANDLE_64(uint8) dirty_bitmap)
> > {
> > long rc = 0;
> > - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> > + struct p2m_domain *p2m = p2m_get_hostp2m(d);
> > + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
> >
> > if ( nr )
> > {
> > @@ -149,7 +153,7 @@ int hap_track_dirty_vram(struct domain *
> >
> > dirty_vram->begin_pfn = begin_pfn;
> > dirty_vram->end_pfn = begin_pfn + nr;
> > - d->arch.hvm_domain.dirty_vram = dirty_vram;
> > + p2m->dirty_vram = dirty_vram;
> > hap_vram_tracking_init(d);
> > rc = paging_log_dirty_enable(d);
> > if (rc != 0)
> > @@ -171,7 +175,7 @@ int hap_track_dirty_vram(struct domain *
> > if ( paging_mode_log_dirty(d) && dirty_vram ) {
> > rc = paging_log_dirty_disable(d);
> > xfree(dirty_vram);
> > - dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
> > + dirty_vram = p2m->dirty_vram = NULL;
> > } else
> > rc = 0;
> > }
> > @@ -182,7 +186,7 @@ param_fail:
> > if ( dirty_vram )
> > {
> > xfree(dirty_vram);
> > - dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
> > + dirty_vram = p2m->dirty_vram = NULL;
> > }
> > return rc;
> > }
> > @@ -228,12 +232,13 @@ static void hap_clean_dirty_bitmap(struc
> >
> > void hap_logdirty_init(struct domain *d)
> > {
> > - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> > + struct p2m_domain *p2m = p2m_get_hostp2m(d);
> > + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
> > if ( paging_mode_log_dirty(d) && dirty_vram )
> > {
> > paging_log_dirty_disable(d);
> > xfree(dirty_vram);
> > - dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
> > + dirty_vram = p2m->dirty_vram = NULL;
> > }
> >
> > /* Reinitialize logdirty mechanism */
> > diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/arch/x86/mm/shadow/common.c
> > --- a/xen/arch/x86/mm/shadow/common.c
> > +++ b/xen/arch/x86/mm/shadow/common.c
> > @@ -3211,11 +3211,11 @@ void shadow_teardown(struct domain *d)
> > * calls now that we've torn down the bitmap */
> > d->arch.paging.mode &= ~PG_log_dirty;
> >
> > - if (d->arch.hvm_domain.dirty_vram) {
> > - xfree(d->arch.hvm_domain.dirty_vram->sl1ma);
> > - xfree(d->arch.hvm_domain.dirty_vram->dirty_bitmap);
> > - xfree(d->arch.hvm_domain.dirty_vram);
> > - d->arch.hvm_domain.dirty_vram = NULL;
> > + if (p2m->dirty_vram) {
> > + xfree(p2m->dirty_vram->sl1ma);
> > + xfree(p2m->dirty_vram->dirty_bitmap);
> > + xfree(p2m->dirty_vram);
> > + p2m->dirty_vram = NULL;
> > }
> >
> > shadow_unlock(d);
> > @@ -3559,8 +3559,8 @@ int shadow_track_dirty_vram(struct domai
> > int flush_tlb = 0;
> > unsigned long i;
> > p2m_type_t t;
> > - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> > struct p2m_domain *p2m = p2m_get_hostp2m(d);
> > + struct sh_dirty_vram *dirty_vram = p2m->dirty_vram;
> >
> > if (end_pfn < begin_pfn
> >
> > || begin_pfn > p2m->max_mapped_pfn
> >
> > @@ -3574,11 +3574,12 @@ int shadow_track_dirty_vram(struct domai
> >
> > || end_pfn != dirty_vram->end_pfn )) )
> >
> > {
> > /* Different tracking, tear the previous down. */
> > - gdprintk(XENLOG_INFO, "stopping tracking VRAM %lx - %lx\n",
> > dirty_vram->begin_pfn, dirty_vram->end_pfn); +
> > gdprintk(XENLOG_INFO, "stopping tracking VRAM %lx - %lx\n", +
> > dirty_vram->begin_pfn, dirty_vram->end_pfn);
> > xfree(dirty_vram->sl1ma);
> > xfree(dirty_vram->dirty_bitmap);
> > xfree(dirty_vram);
> > - dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
> > + dirty_vram = p2m->dirty_vram = NULL;
> > }
> >
> > if ( !nr )
> > @@ -3602,7 +3603,7 @@ int shadow_track_dirty_vram(struct domai
> > goto out;
> > dirty_vram->begin_pfn = begin_pfn;
> > dirty_vram->end_pfn = end_pfn;
> > - d->arch.hvm_domain.dirty_vram = dirty_vram;
> > + p2m->dirty_vram = dirty_vram;
> >
> > if ( (dirty_vram->sl1ma = xmalloc_array(paddr_t, nr)) == NULL )
> > goto out_dirty_vram;
> > @@ -3735,7 +3736,7 @@ out_sl1ma:
> > xfree(dirty_vram->sl1ma);
> > out_dirty_vram:
> > xfree(dirty_vram);
> > - dirty_vram = d->arch.hvm_domain.dirty_vram = NULL;
> > + dirty_vram = p2m->dirty_vram = NULL;
> >
> > out:
> > shadow_unlock(d);
> > diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/arch/x86/mm/shadow/multi.c
> > --- a/xen/arch/x86/mm/shadow/multi.c
> > +++ b/xen/arch/x86/mm/shadow/multi.c
> > @@ -515,7 +515,7 @@ _sh_propagate(struct vcpu *v,
> > guest_l1e_t guest_entry = { guest_intpte };
> > shadow_l1e_t *sp = shadow_entry_ptr;
> > struct domain *d = v->domain;
> > - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> > + struct sh_dirty_vram *dirty_vram = p2m_get_hostp2m(d)->dirty_vram;
> > gfn_t target_gfn = guest_l1e_get_gfn(guest_entry);
> > u32 pass_thru_flags;
> > u32 gflags, sflags;
> > @@ -1105,7 +1105,7 @@ static inline void shadow_vram_get_l1e(s
> > mfn_t mfn = shadow_l1e_get_mfn(new_sl1e);
> > int flags = shadow_l1e_get_flags(new_sl1e);
> > unsigned long gfn;
> > - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> > + struct sh_dirty_vram *dirty_vram = p2m_get_hostp2m(d)->dirty_vram;
> >
> > if ( !dirty_vram /* tracking disabled? */
> >
> > || !(flags & _PAGE_RW) /* read-only mapping? */
> >
> > @@ -1136,7 +1136,7 @@ static inline void shadow_vram_put_l1e(s
> > mfn_t mfn = shadow_l1e_get_mfn(old_sl1e);
> > int flags = shadow_l1e_get_flags(old_sl1e);
> > unsigned long gfn;
> > - struct sh_dirty_vram *dirty_vram = d->arch.hvm_domain.dirty_vram;
> > + struct sh_dirty_vram *dirty_vram = p2m_get_hostp2m(d)->dirty_vram;
> >
> > if ( !dirty_vram /* tracking disabled? */
> >
> > || !(flags & _PAGE_RW) /* read-only mapping? */
> >
> > diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/include/asm-x86/hvm/domain.h
> > --- a/xen/include/asm-x86/hvm/domain.h
> > +++ b/xen/include/asm-x86/hvm/domain.h
> > @@ -69,9 +69,6 @@ struct hvm_domain {
> > /* Memory ranges with pinned cache attributes. */
> > struct list_head pinned_cacheattr_ranges;
> >
> > - /* VRAM dirty support. */
> > - struct sh_dirty_vram *dirty_vram;
> > -
> > /* If one of vcpus of this domain is in no_fill_mode or
> > * mtrr/pat between vcpus is not the same, set is_in_uc_mode
> > */
> > diff -r c1a95c7ef858 -r 50b3e6c73d7c xen/include/asm-x86/p2m.h
> > --- a/xen/include/asm-x86/p2m.h
> > +++ b/xen/include/asm-x86/p2m.h
> > @@ -172,6 +172,9 @@ struct p2m_domain {
> > /* Shadow translated domain: p2m mapping */
> > pagetable_t phys_table;
> >
> > + /* VRAM dirty support. */
> > + struct sh_dirty_vram *dirty_vram;
> > +
> > struct domain *domain; /* back pointer to domain */
> >
> > /* Pages used to construct the p2m */
>
> --
> Tim Deegan <Tim.Deegan@citrix.com>
> Principal Software Engineer, XenServer Engineering
> Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
--
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: Re: [PATCH 12/13] Nested Virtualization: vram
2010-09-08 15:47 ` Christoph Egger
@ 2010-09-13 13:16 ` Tim Deegan
2010-09-13 13:32 ` Christoph Egger
0 siblings, 1 reply; 10+ messages in thread
From: Tim Deegan @ 2010-09-13 13:16 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel@lists.xensource.com, Dong, Eddie
At 16:47 +0100 on 08 Sep (1283964447), Christoph Egger wrote:
> On Wednesday 08 September 2010 17:07:35 Tim Deegan wrote:
> > Hi,
> >
> > Is this needed as part of the nested-HVM series or just an independent
> > interface change?
>
> This is an independent interface change.
> It is not needed to make nested-HVM work in general but it is part of
> a larger change to make log dirty code nested virtualization aware.
Ah; does the current nested-SVM patchset break logdirty?
I don't think making the vram structures per-P2M is the best approach.
We're never going to have more than one vram area to track per guest so
it can just operate on the host-p2m, like it does already.
In general, the log-dirty code operates on N1 pfns, and we won't want a
per-p2m log-dirty bitmap either; we'd only have to fold them together to
use them in the tools.
Cheers,
Tim.
--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: [PATCH 12/13] Nested Virtualization: vram
2010-09-13 13:16 ` Tim Deegan
@ 2010-09-13 13:32 ` Christoph Egger
2010-09-13 13:50 ` Tim Deegan
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Egger @ 2010-09-13 13:32 UTC (permalink / raw)
To: Tim Deegan; +Cc: xen-devel@lists.xensource.com, Dong, Eddie
On Monday 13 September 2010 15:16:41 Tim Deegan wrote:
> At 16:47 +0100 on 08 Sep (1283964447), Christoph Egger wrote:
> > On Wednesday 08 September 2010 17:07:35 Tim Deegan wrote:
> > > Hi,
> > >
> > > Is this needed as part of the nested-HVM series or just an independent
> > > interface change?
> >
> > This is an independent interface change.
> > It is not needed to make nested-HVM work in general but it is part of
> > a larger change to make log dirty code nested virtualization aware.
>
> Ah; does the current nested-SVM patchset break logdirty?
I would say hap-on-hap in general does.
I mean that also counts for ept-on-ept.
See below.
>
> I don't think making the vram structures per-P2M is the best approach.
> We're never going to have more than one vram area to track per guest so
> it can just operate on the host-p2m, like it does already.
>
> In general, the log-dirty code operates on N1 pfns, and we won't want a
> per-p2m log-dirty bitmap either; we'd only have to fold them together to
> use them in the tools.
Look at this trace:
(XEN) [<ffff82c4801f953e>] hap_write_p2m_entry+0x3e/0x1cb
(XEN) [<ffff82c4801cf285>] p2m_set_entry+0x4a7/0x782
(XEN) [<ffff82c4801c88e1>] set_p2m_entry+0xb3/0x101
(XEN) [<ffff82c4801cba46>] p2m_change_type+0x120/0x17a
(XEN) [<ffff82c4801f94ce>] hap_clean_vram_tracking+0x44/0x76
(XEN) [<ffff82c4801c7a6e>] paging_log_dirty_range+0x33/0x8b4
(XEN) [<ffff82c4801f9420>] hap_track_dirty_vram+0x109/0x173
(XEN) [<ffff82c4801a7afe>] do_hvm_op+0xc1a/0x12a5
(XEN) [<ffff82c4802000d2>] syscall_enter+0xf2/0x14c
The problem is in paging_write_p2m_entry():
struct vcpu *v = current;
if ( v->domain != d )
v = d->vcpu ? d->vcpu[0] ? NULL;
The chosen vcpu can be in guest mode and fill the vram / logdirty
host p2m with l2 guest related data.
Christoph
--
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: Re: [PATCH 12/13] Nested Virtualization: vram
2010-09-13 13:32 ` Christoph Egger
@ 2010-09-13 13:50 ` Tim Deegan
2010-09-13 15:36 ` Christoph Egger
0 siblings, 1 reply; 10+ messages in thread
From: Tim Deegan @ 2010-09-13 13:50 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel@lists.xensource.com, Dong, Eddie
At 14:32 +0100 on 13 Sep (1284388352), Christoph Egger wrote:
> > I don't think making the vram structures per-P2M is the best approach.
> > We're never going to have more than one vram area to track per guest so
> > it can just operate on the host-p2m, like it does already.
> >
> > In general, the log-dirty code operates on N1 pfns, and we won't want a
> > per-p2m log-dirty bitmap either; we'd only have to fold them together to
> > use them in the tools.
>
> Look at this trace:
>
> (XEN) [<ffff82c4801f953e>] hap_write_p2m_entry+0x3e/0x1cb
> (XEN) [<ffff82c4801cf285>] p2m_set_entry+0x4a7/0x782
> (XEN) [<ffff82c4801c88e1>] set_p2m_entry+0xb3/0x101
> (XEN) [<ffff82c4801cba46>] p2m_change_type+0x120/0x17a
> (XEN) [<ffff82c4801f94ce>] hap_clean_vram_tracking+0x44/0x76
> (XEN) [<ffff82c4801c7a6e>] paging_log_dirty_range+0x33/0x8b4
> (XEN) [<ffff82c4801f9420>] hap_track_dirty_vram+0x109/0x173
> (XEN) [<ffff82c4801a7afe>] do_hvm_op+0xc1a/0x12a5
> (XEN) [<ffff82c4802000d2>] syscall_enter+0xf2/0x14c
>
> The problem is in paging_write_p2m_entry():
>
> struct vcpu *v = current;
> if ( v->domain != d )
> v = d->vcpu ? d->vcpu[0] ? NULL;
>
> The chosen vcpu can be in guest mode and fill the vram / logdirty
> host p2m with l2 guest related data.
OK. That's certainly confusing. I think the fix is to have all the
outward-facing interfaces to the p2m code always operate on the host
(L1->L0) p2m. None of their callers would know what to do with an L2
pfn anyway. Only code that explicitly asks for it (e.g. the NPF
handler) should see the L2->L0 p2m.
Cheers,
Tim.
--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: [PATCH 12/13] Nested Virtualization: vram
2010-09-13 13:50 ` Tim Deegan
@ 2010-09-13 15:36 ` Christoph Egger
2010-09-13 15:51 ` Tim Deegan
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Egger @ 2010-09-13 15:36 UTC (permalink / raw)
To: Tim Deegan; +Cc: xen-devel@lists.xensource.com, Dong, Eddie
On Monday 13 September 2010 15:50:40 Tim Deegan wrote:
> At 14:32 +0100 on 13 Sep (1284388352), Christoph Egger wrote:
> > > I don't think making the vram structures per-P2M is the best approach.
> > > We're never going to have more than one vram area to track per guest so
> > > it can just operate on the host-p2m, like it does already.
> > >
> > > In general, the log-dirty code operates on N1 pfns, and we won't want a
> > > per-p2m log-dirty bitmap either; we'd only have to fold them together
> > > to use them in the tools.
> >
> > Look at this trace:
> >
> > (XEN) [<ffff82c4801f953e>] hap_write_p2m_entry+0x3e/0x1cb
> > (XEN) [<ffff82c4801cf285>] p2m_set_entry+0x4a7/0x782
> > (XEN) [<ffff82c4801c88e1>] set_p2m_entry+0xb3/0x101
> > (XEN) [<ffff82c4801cba46>] p2m_change_type+0x120/0x17a
> > (XEN) [<ffff82c4801f94ce>] hap_clean_vram_tracking+0x44/0x76
> > (XEN) [<ffff82c4801c7a6e>] paging_log_dirty_range+0x33/0x8b4
> > (XEN) [<ffff82c4801f9420>] hap_track_dirty_vram+0x109/0x173
> > (XEN) [<ffff82c4801a7afe>] do_hvm_op+0xc1a/0x12a5
> > (XEN) [<ffff82c4802000d2>] syscall_enter+0xf2/0x14c
> >
> > The problem is in paging_write_p2m_entry():
> >
> > struct vcpu *v = current;
> > if ( v->domain != d )
> > v = d->vcpu ? d->vcpu[0] ? NULL;
> >
> > The chosen vcpu can be in guest mode and fill the vram / logdirty
> > host p2m with l2 guest related data.
>
> OK. That's certainly confusing. I think the fix is to have all the
> outward-facing interfaces to the p2m code always operate on the host
> (L1->L0) p2m. None of their callers would know what to do with an L2
> pfn anyway. Only code that explicitly asks for it (e.g. the NPF
> handler) should see the L2->L0 p2m.
The instruction emulator also must see the L2 -> L0 p2m
- to be more precise it is __hvm_copy() that fetches the
instruction - in order to be able to emulate instructions
for the L2 guest the L1 guest does not intercept.
Christoph
--
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: [PATCH 12/13] Nested Virtualization: vram
2010-09-13 15:36 ` Christoph Egger
@ 2010-09-13 15:51 ` Tim Deegan
2010-09-13 16:08 ` Christoph Egger
0 siblings, 1 reply; 10+ messages in thread
From: Tim Deegan @ 2010-09-13 15:51 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel@lists.xensource.com, Dong, Eddie
At 16:36 +0100 on 13 Sep (1284395780), Christoph Egger wrote:
> On Monday 13 September 2010 15:50:40 Tim Deegan wrote:
> > OK. That's certainly confusing. I think the fix is to have all the
> > outward-facing interfaces to the p2m code always operate on the host
> > (L1->L0) p2m. None of their callers would know what to do with an L2
> > pfn anyway. Only code that explicitly asks for it (e.g. the NPF
> > handler) should see the L2->L0 p2m.
>
> The instruction emulator also must see the L2 -> L0 p2m
> - to be more precise it is __hvm_copy() that fetches the
> instruction - in order to be able to emulate instructions
> for the L2 guest the L1 guest does not intercept.
It needs to be able to fetch from virtual addresses; if we make
paging_gva_to_gfn always return an N1 pfn then that should just work.
Does the instruction emulator need to read N2 pfns for anything else?
Tim.
--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: [PATCH 12/13] Nested Virtualization: vram
2010-09-13 15:51 ` Tim Deegan
@ 2010-09-13 16:08 ` Christoph Egger
2010-09-13 16:17 ` Tim Deegan
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Egger @ 2010-09-13 16:08 UTC (permalink / raw)
To: Tim Deegan; +Cc: xen-devel@lists.xensource.com, Dong, Eddie
On Monday 13 September 2010 17:51:15 Tim Deegan wrote:
> At 16:36 +0100 on 13 Sep (1284395780), Christoph Egger wrote:
> > On Monday 13 September 2010 15:50:40 Tim Deegan wrote:
> > > OK. That's certainly confusing. I think the fix is to have all the
> > > outward-facing interfaces to the p2m code always operate on the host
> > > (L1->L0) p2m. None of their callers would know what to do with an L2
> > > pfn anyway. Only code that explicitly asks for it (e.g. the NPF
> > > handler) should see the L2->L0 p2m.
> >
> > The instruction emulator also must see the L2 -> L0 p2m
> > - to be more precise it is __hvm_copy() that fetches the
> > instruction - in order to be able to emulate instructions
> > for the L2 guest the L1 guest does not intercept.
>
> It needs to be able to fetch from virtual addresses; if we make
> paging_gva_to_gfn always return an N1 pfn then that should just work.
> Does the instruction emulator need to read N2 pfns for anything else?
The L2 -> L0 p2m (= nestedp2m) is needed to translate the L2 guest's
cr3 into host physical address in order to be able to walk the L2 guest's
page table. Then we have the L2 guest physical address where the
instruction sits. Then the instruction emulator is invoked and operates
as usual with the difference that the nestedp2m is used instead of the
hostp2m.
Christoph
--
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: [PATCH 12/13] Nested Virtualization: vram
2010-09-13 16:08 ` Christoph Egger
@ 2010-09-13 16:17 ` Tim Deegan
0 siblings, 0 replies; 10+ messages in thread
From: Tim Deegan @ 2010-09-13 16:17 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel@lists.xensource.com, Dong, Eddie
At 17:08 +0100 on 13 Sep (1284397714), Christoph Egger wrote:
> On Monday 13 September 2010 17:51:15 Tim Deegan wrote:
> > At 16:36 +0100 on 13 Sep (1284395780), Christoph Egger wrote:
> > > On Monday 13 September 2010 15:50:40 Tim Deegan wrote:
> > > > OK. That's certainly confusing. I think the fix is to have all the
> > > > outward-facing interfaces to the p2m code always operate on the host
> > > > (L1->L0) p2m. None of their callers would know what to do with an L2
> > > > pfn anyway. Only code that explicitly asks for it (e.g. the NPF
> > > > handler) should see the L2->L0 p2m.
> > >
> > > The instruction emulator also must see the L2 -> L0 p2m
> > > - to be more precise it is __hvm_copy() that fetches the
> > > instruction - in order to be able to emulate instructions
> > > for the L2 guest the L1 guest does not intercept.
> >
> > It needs to be able to fetch from virtual addresses; if we make
> > paging_gva_to_gfn always return an N1 pfn then that should just work.
> > Does the instruction emulator need to read N2 pfns for anything else?
>
> The L2 -> L0 p2m (= nestedp2m) is needed to translate the L2 guest's
> cr3 into host physical address in order to be able to walk the L2 guest's
> page table.
Yes, the HAP pagetable walker will definitely need to be aware of the
N2->N0 p2m, so that it can translate CR3 and all the pagetable entries.
My suggestion is that when it's done that it might as well translate the
final physical address into N1-pfn-space so that its callers don't have
to know about nested HAP.
If that's too expensive (since N2->N1 lookup could involve multiple
N1->N0 lookups), maybe a gva_to_mfn() function that goes straight to
MFNs would be a useful interface?
> Then we have the L2 guest physical address where the
> instruction sits. Then the instruction emulator is invoked and operates
> as usual with the difference that the nestedp2m is used instead of the
> hostp2m.
The instruction emulator deals almost entirely in virtual addresses,
except for a few places in the accessor callbacks, and I think those
places could (and should) use N1 pfns, not N2.
Tim.
> Christoph
>
>
> --
> ---to satisfy European Law for business letters:
> Advanced Micro Devices GmbH
> Einsteinring 24, 85609 Dornach b. Muenchen
> Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
> Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
> Registergericht Muenchen, HRB Nr. 43632
>
--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-09-13 16:17 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-01 15:17 [PATCH 12/13] Nested Virtualization: vram Christoph Egger
2010-09-08 15:07 ` Tim Deegan
2010-09-08 15:47 ` Christoph Egger
2010-09-13 13:16 ` Tim Deegan
2010-09-13 13:32 ` Christoph Egger
2010-09-13 13:50 ` Tim Deegan
2010-09-13 15:36 ` Christoph Egger
2010-09-13 15:51 ` Tim Deegan
2010-09-13 16:08 ` Christoph Egger
2010-09-13 16:17 ` Tim Deegan
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.