Re: [Xen-devel] [PATCH 8/8] viridian: introduce struct viridian_page
> -Original Message- > From: Roger Pau Monne > Sent: 30 October 2018 17:18 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Wei Liu ; Ian > Jackson ; Jan Beulich ; Andrew > Cooper > Subject: Re: [Xen-devel] [PATCH 8/8] viridian: introduce struct > viridian_page > > On Mon, Oct 29, 2018 at 06:02:11PM +, Paul Durrant wrote: > > The 'vp_assist' page is currently an example of a guest page which needs > to > > be kept mapped throughout the life-time of a guest, but there are other > > such examples in the specifiction [1]. This patch therefore introduces a > > generic 'viridian_page' type and converts the current > vp_assist/apic_assist > > related code to use it. Subsequent patches implementing other > enlightments > > can then also make use of it. > > > > This patch also renames the 'vp_assist_pending' field in struct > > hvm_viridian_vcpu_context to 'apic_assist_pending' to more accurately > > reflect its meaning. The term 'vp_assist' applies to the whole page > rather > > than just the EOI-avoidance enlightenment. New versons of the > specification > > have defined data structures for other enlightenments within the same > page. > > > > No functional change. > > > > [1] https://github.com/MicrosoftDocs/Virtualization- > Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specif > ication%20v5.0C.pdf > > > > Signed-off-by: Paul Durrant > > Reviewed-by: Roger Pau Monné > Thanks. > > --- > > Cc: Ian Jackson > > Cc: Wei Liu > > Cc: Jan Beulich > > Cc: Andrew Cooper > > --- > > tools/misc/xen-hvmctx.c| 4 +-- > > xen/arch/x86/hvm/viridian/synic.c | 47 --- > --- > > xen/include/asm-x86/hvm/viridian.h | 13 ++ > > xen/include/public/arch-x86/hvm/save.h | 2 +- > > 4 files changed, 33 insertions(+), 33 deletions(-) > > > > diff --git a/tools/misc/xen-hvmctx.c b/tools/misc/xen-hvmctx.c > > index 40e77851be..a4efadf307 100644 > > --- a/tools/misc/xen-hvmctx.c > > +++ b/tools/misc/xen-hvmctx.c > > @@ -370,9 +370,9 @@ static void dump_viridian_vcpu(void) > > { > > HVM_SAVE_TYPE(VIRIDIAN_VCPU) p; > > READ(p); > > -printf("VIRIDIAN_VCPU: vp_assist_msr 0x%llx, vp_assist_pending > %s\n", > > +printf("VIRIDIAN_VCPU: vp_assist_msr 0x%llx, > apic_assist_pending %s\n", > >(unsigned long long) p.vp_assist_msr, > > - p.vp_assist_pending ? "true" : "false"); > > + p.apic_assist_pending ? "true" : "false"); > > There's some hardcoded tabs in the above line, would you mind fixing > them since you are already touching this code? Sure, I'll take a look at the whitespace. Paul > > Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 8/8] viridian: introduce struct viridian_page
On Mon, Oct 29, 2018 at 06:02:11PM +, Paul Durrant wrote: > The 'vp_assist' page is currently an example of a guest page which needs to > be kept mapped throughout the life-time of a guest, but there are other > such examples in the specifiction [1]. This patch therefore introduces a > generic 'viridian_page' type and converts the current vp_assist/apic_assist > related code to use it. Subsequent patches implementing other enlightments > can then also make use of it. > > This patch also renames the 'vp_assist_pending' field in struct > hvm_viridian_vcpu_context to 'apic_assist_pending' to more accurately > reflect its meaning. The term 'vp_assist' applies to the whole page rather > than just the EOI-avoidance enlightenment. New versons of the specification > have defined data structures for other enlightenments within the same page. > > No functional change. > > [1] > https://github.com/MicrosoftDocs/Virtualization-Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specification%20v5.0C.pdf > > Signed-off-by: Paul Durrant Reviewed-by: Roger Pau Monné > --- > Cc: Ian Jackson > Cc: Wei Liu > Cc: Jan Beulich > Cc: Andrew Cooper > --- > tools/misc/xen-hvmctx.c| 4 +-- > xen/arch/x86/hvm/viridian/synic.c | 47 > -- > xen/include/asm-x86/hvm/viridian.h | 13 ++ > xen/include/public/arch-x86/hvm/save.h | 2 +- > 4 files changed, 33 insertions(+), 33 deletions(-) > > diff --git a/tools/misc/xen-hvmctx.c b/tools/misc/xen-hvmctx.c > index 40e77851be..a4efadf307 100644 > --- a/tools/misc/xen-hvmctx.c > +++ b/tools/misc/xen-hvmctx.c > @@ -370,9 +370,9 @@ static void dump_viridian_vcpu(void) > { > HVM_SAVE_TYPE(VIRIDIAN_VCPU) p; > READ(p); > -printf("VIRIDIAN_VCPU: vp_assist_msr 0x%llx, vp_assist_pending %s\n", > +printf("VIRIDIAN_VCPU: vp_assist_msr 0x%llx, apic_assist_pending > %s\n", > (unsigned long long) p.vp_assist_msr, > -p.vp_assist_pending ? "true" : "false"); > +p.apic_assist_pending ? "true" : "false"); There's some hardcoded tabs in the above line, would you mind fixing them since you are already touching this code? Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 8/8] viridian: introduce struct viridian_page
The 'vp_assist' page is currently an example of a guest page which needs to be kept mapped throughout the life-time of a guest, but there are other such examples in the specifiction [1]. This patch therefore introduces a generic 'viridian_page' type and converts the current vp_assist/apic_assist related code to use it. Subsequent patches implementing other enlightments can then also make use of it. This patch also renames the 'vp_assist_pending' field in struct hvm_viridian_vcpu_context to 'apic_assist_pending' to more accurately reflect its meaning. The term 'vp_assist' applies to the whole page rather than just the EOI-avoidance enlightenment. New versons of the specification have defined data structures for other enlightenments within the same page. No functional change. [1] https://github.com/MicrosoftDocs/Virtualization-Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specification%20v5.0C.pdf Signed-off-by: Paul Durrant --- Cc: Ian Jackson Cc: Wei Liu Cc: Jan Beulich Cc: Andrew Cooper --- tools/misc/xen-hvmctx.c| 4 +-- xen/arch/x86/hvm/viridian/synic.c | 47 -- xen/include/asm-x86/hvm/viridian.h | 13 ++ xen/include/public/arch-x86/hvm/save.h | 2 +- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/tools/misc/xen-hvmctx.c b/tools/misc/xen-hvmctx.c index 40e77851be..a4efadf307 100644 --- a/tools/misc/xen-hvmctx.c +++ b/tools/misc/xen-hvmctx.c @@ -370,9 +370,9 @@ static void dump_viridian_vcpu(void) { HVM_SAVE_TYPE(VIRIDIAN_VCPU) p; READ(p); -printf("VIRIDIAN_VCPU: vp_assist_msr 0x%llx, vp_assist_pending %s\n", +printf("VIRIDIAN_VCPU: vp_assist_msr 0x%llx, apic_assist_pending %s\n", (unsigned long long) p.vp_assist_msr, - p.vp_assist_pending ? "true" : "false"); + p.apic_assist_pending ? "true" : "false"); } static void dump_vmce_vcpu(void) diff --git a/xen/arch/x86/hvm/viridian/synic.c b/xen/arch/x86/hvm/viridian/synic.c index 735c4c897c..f9aa8fd898 100644 --- a/xen/arch/x86/hvm/viridian/synic.c +++ b/xen/arch/x86/hvm/viridian/synic.c @@ -36,14 +36,13 @@ static void dump_vp_assist(const struct vcpu *v) v, (unsigned long)va->fields.pfn); } -static void initialize_vp_assist(struct vcpu *v) +static void initialize_guest_page(struct vcpu *v, struct viridian_page *vp) { struct domain *d = v->domain; -unsigned long gmfn = v->arch.hvm.viridian.vp_assist.msr.fields.pfn; +unsigned long gmfn = vp->msr.fields.pfn; struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); -HV_VP_ASSIST_PAGE *ptr; -ASSERT(!v->arch.hvm.viridian.vp_assist.ptr); +ASSERT(!vp->ptr); if ( !page ) goto fail; @@ -54,16 +53,14 @@ static void initialize_vp_assist(struct vcpu *v) goto fail; } -ptr = __map_domain_page_global(page); -if ( !ptr ) +vp->ptr = __map_domain_page_global(page); +if ( !vp->ptr ) { put_page_and_type(page); goto fail; } -clear_page(ptr); - -v->arch.hvm.viridian.vp_assist.ptr = ptr; +clear_page(vp->ptr); return; fail: @@ -71,19 +68,18 @@ static void initialize_vp_assist(struct vcpu *v) gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); } -static void teardown_vp_assist(struct vcpu *v) +static void teardown_guest_page(struct viridian_page *vp) { -HV_VP_ASSIST_PAGE *ptr = v->arch.hvm.viridian.vp_assist.ptr; struct page_info *page; -if ( !ptr ) +if ( !vp->ptr ) return; -v->arch.hvm.viridian.vp_assist.ptr = NULL; +page = mfn_to_page(domain_page_map_to_mfn(vp->ptr)); -page = mfn_to_page(domain_page_map_to_mfn(ptr)); +unmap_domain_page_global(vp->ptr); +vp->ptr = NULL; -unmap_domain_page_global(ptr); put_page_and_type(page); } @@ -99,10 +95,10 @@ void viridian_apic_assist_set(struct vcpu *v) * wrong and the VM will most likely hang so force a crash now * to make the problem clear. */ -if ( v->arch.hvm.viridian.vp_assist.pending ) +if ( v->arch.hvm.viridian.apic_assist_pending ) domain_crash(v->domain); -v->arch.hvm.viridian.vp_assist.pending = true; +v->arch.hvm.viridian.apic_assist_pending = true; ptr->ApicAssist.no_eoi = 1; } @@ -113,11 +109,11 @@ bool viridian_apic_assist_completed(struct vcpu *v) if ( !ptr ) return false; -if ( v->arch.hvm.viridian.vp_assist.pending && +if ( v->arch.hvm.viridian.apic_assist_pending && !ptr->ApicAssist.no_eoi ) { /* An EOI has been avoided */ -v->arch.hvm.viridian.vp_assist.pending = false; +v->arch.hvm.viridian.apic_assist_pending = false; return true; } @@ -132,7 +128,7 @@ void viridian_apic_assist_clear(struct vcpu *v) return; ptr->ApicAssist.no_eoi = 0; -v->arch.hvm.viridian.vp_assist.pending = false; +v->arch.hvm.virid