| Message ID | 1490120242-3587-7-git-send-email-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 21.03.17 at 19:17, <paul.durrant@citrix.com> wrote: > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -288,6 +288,12 @@ > #define LIBXL_HAVE_SCHED_CREDIT2_PARAMS 1 > > /* > + * LIBXL_HAVE_CRASH_CTL indicates that the 'crash_ctl' value > + * is present in the viridian enlightenment enumeration. > + */ > +#define LIBXL_HAVE_CRASH_CTL 1 I was about to commit this together with 1, 3, and 4, when this define caught my eye: Isn't the name a little too generic? I.e. LIBXL_HAVE_VIRIDIAN_CRASH_CTL perhaps? Jan
On Wed, Mar 22, 2017 at 05:07:30AM -0600, Jan Beulich wrote: > >>> On 21.03.17 at 19:17, <paul.durrant@citrix.com> wrote: > > --- a/tools/libxl/libxl.h > > +++ b/tools/libxl/libxl.h > > @@ -288,6 +288,12 @@ > > #define LIBXL_HAVE_SCHED_CREDIT2_PARAMS 1 > > > > /* > > + * LIBXL_HAVE_CRASH_CTL indicates that the 'crash_ctl' value > > + * is present in the viridian enlightenment enumeration. > > + */ > > +#define LIBXL_HAVE_CRASH_CTL 1 > > I was about to commit this together with 1, 3, and 4, when this > define caught my eye: Isn't the name a little too generic? I.e. > LIBXL_HAVE_VIRIDIAN_CRASH_CTL perhaps? > That's fine by me. > Jan >
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@citrix.com] > Sent: 22 March 2017 11:10 > To: Jan Beulich <JBeulich@suse.com> > Cc: Paul Durrant <Paul.Durrant@citrix.com>; Wei Liu <wei.liu2@citrix.com>; > Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson > <Ian.Jackson@citrix.com>; xen-devel@lists.xenproject.org > Subject: Re: [Xen-devel] [PATCH v3 6/6] x86/viridian: implement the crash > MSRs > > On Wed, Mar 22, 2017 at 05:07:30AM -0600, Jan Beulich wrote: > > >>> On 21.03.17 at 19:17, <paul.durrant@citrix.com> wrote: > > > --- a/tools/libxl/libxl.h > > > +++ b/tools/libxl/libxl.h > > > @@ -288,6 +288,12 @@ > > > #define LIBXL_HAVE_SCHED_CREDIT2_PARAMS 1 > > > > > > /* > > > + * LIBXL_HAVE_CRASH_CTL indicates that the 'crash_ctl' value > > > + * is present in the viridian enlightenment enumeration. > > > + */ > > > +#define LIBXL_HAVE_CRASH_CTL 1 > > > > I was about to commit this together with 1, 3, and 4, when this > > define caught my eye: Isn't the name a little too generic? I.e. > > LIBXL_HAVE_VIRIDIAN_CRASH_CTL perhaps? > > > > That's fine by me. > Ok with me too. Paul > > Jan > >
>>> On 21.03.17 at 19:17, <paul.durrant@citrix.com> wrote: > + case HV_X64_MSR_CRASH_CTL: > + { > + HV_CRASH_CTL_REG_CONTENTS ctl = { > + .CrashNotify = 1, > + }; This initializer failed my pre-push build test: Some gcc versions we still support don't allow fields of unnamed union members to be initialized this way. As there are at least two ways to deal with this, and as this is code maintained by you, I didn't want to pick either, as I don't know your preference. Jan
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 22 March 2017 11:38 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson > <Ian.Jackson@citrix.com>; xen-devel@lists.xenproject.org > Subject: Re: [Xen-devel] [PATCH v3 6/6] x86/viridian: implement the crash > MSRs > > >>> On 21.03.17 at 19:17, <paul.durrant@citrix.com> wrote: > > + case HV_X64_MSR_CRASH_CTL: > > + { > > + HV_CRASH_CTL_REG_CONTENTS ctl = { > > + .CrashNotify = 1, > > + }; > > This initializer failed my pre-push build test: Some gcc versions we > still support don't allow fields of unnamed union members to be > initialized this way. As there are at least two ways to deal with > this, and as this is code maintained by you, I didn't want to pick > either, as I don't know your preference. > Ok, I guess I'll go for naming the union member. Do you want me to resubmit? Paul > Jan
diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in index 52802d5..991960b 100644 --- a/docs/man/xl.cfg.pod.5.in +++ b/docs/man/xl.cfg.pod.5.in @@ -1601,11 +1601,17 @@ per-vcpu event channel upcall vectors. Note that this enlightenment will have no effect if the guest is using APICv posted interrupts. +=item B<crash_ctl> + +This group incorporates the crash control MSRs. These enlightenments +allow Windows to write crash information such that it can be logged +by Xen. + =item B<defaults> This is a special value that enables the default set of groups, which -is currently the B<base>, B<freq>, B<time_ref_count> and B<apic_assist> -groups. +is currently the B<base>, B<freq>, B<time_ref_count>, B<apic_assist> +and B<crash_ctl> groups. =item B<all> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 72ec39d..af45582 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -288,6 +288,12 @@ #define LIBXL_HAVE_SCHED_CREDIT2_PARAMS 1 /* + * LIBXL_HAVE_CRASH_CTL indicates that the 'crash_ctl' value + * is present in the viridian enlightenment enumeration. + */ +#define LIBXL_HAVE_CRASH_CTL 1 + +/* * libxl ABI compatibility * * The only guarantee which libxl makes regarding ABI compatibility diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index e133962..c25cf48 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -214,6 +214,7 @@ static int hvm_set_viridian_features(libxl__gc *gc, uint32_t domid, libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_FREQ); libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_TIME_REF_COUNT); libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST); + libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_CRASH_CTL); } libxl_for_each_set_bit(v, info->u.hvm.viridian_enable) { @@ -259,6 +260,9 @@ static int hvm_set_viridian_features(libxl__gc *gc, uint32_t domid, if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST)) mask |= HVMPV_apic_assist; + if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_CRASH_CTL)) + mask |= HVMPV_crash_ctl; + if (mask != 0 && xc_hvm_param_set(CTX->xch, domid, diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 2475a4d..69e789a 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -224,6 +224,7 @@ libxl_viridian_enlightenment = Enumeration("viridian_enlightenment", [ (3, "reference_tsc"), (4, "hcall_remote_tlb_flush"), (5, "apic_assist"), + (6, "crash_ctl"), ]) libxl_hdtype = Enumeration("hdtype", [ diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c index 9d1272a..dff749b 100644 --- a/xen/arch/x86/hvm/viridian.c +++ b/xen/arch/x86/hvm/viridian.c @@ -154,6 +154,19 @@ typedef struct { uint64_t Reserved8:10; } HV_PARTITION_PRIVILEGE_MASK; +typedef union _HV_CRASH_CTL_REG_CONTENTS +{ + uint64_t AsUINT64; + struct + { + uint64_t Reserved:63; + uint64_t CrashNotify:1; + }; +} HV_CRASH_CTL_REG_CONTENTS; + +/* Viridian CPUID leaf 3, Hypervisor Feature Indication */ +#define CPUID3D_CRASH_MSRS (1 << 10) + /* Viridian CPUID leaf 4: Implementation Recommendations. */ #define CPUID4A_HCALL_REMOTE_TLB_FLUSH (1 << 2) #define CPUID4A_MSR_BASED_APIC (1 << 3) @@ -249,6 +262,10 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf, res->a = u.lo; res->b = u.hi; + + if ( viridian_feature_mask(d) & HVMPV_crash_ctl ) + res->d = CPUID3D_CRASH_MSRS; + break; } @@ -612,6 +629,36 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t val) update_reference_tsc(d, 1); break; + case HV_X64_MSR_CRASH_P0: + case HV_X64_MSR_CRASH_P1: + case HV_X64_MSR_CRASH_P2: + case HV_X64_MSR_CRASH_P3: + case HV_X64_MSR_CRASH_P4: + BUILD_BUG_ON(HV_X64_MSR_CRASH_P4 - HV_X64_MSR_CRASH_P0 >= + ARRAY_SIZE(v->arch.hvm_vcpu.viridian.crash_param)); + + idx -= HV_X64_MSR_CRASH_P0; + v->arch.hvm_vcpu.viridian.crash_param[idx] = val; + break; + + case HV_X64_MSR_CRASH_CTL: + { + HV_CRASH_CTL_REG_CONTENTS ctl; + + ctl.AsUINT64 = val; + + if ( !ctl.CrashNotify ) + break; + + gprintk(XENLOG_WARNING, "VIRIDIAN CRASH: %lx %lx %lx %lx %lx\n", + v->arch.hvm_vcpu.viridian.crash_param[0], + v->arch.hvm_vcpu.viridian.crash_param[1], + v->arch.hvm_vcpu.viridian.crash_param[2], + v->arch.hvm_vcpu.viridian.crash_param[3], + v->arch.hvm_vcpu.viridian.crash_param[4]); + break; + } + default: if ( idx >= VIRIDIAN_MSR_MIN && idx <= VIRIDIAN_MSR_MAX ) gprintk(XENLOG_WARNING, "write to unimplemented MSR %#x\n", @@ -739,6 +786,28 @@ int rdmsr_viridian_regs(uint32_t idx, uint64_t *val) break; } + case HV_X64_MSR_CRASH_P0: + case HV_X64_MSR_CRASH_P1: + case HV_X64_MSR_CRASH_P2: + case HV_X64_MSR_CRASH_P3: + case HV_X64_MSR_CRASH_P4: + BUILD_BUG_ON(HV_X64_MSR_CRASH_P4 - HV_X64_MSR_CRASH_P0 >= + ARRAY_SIZE(v->arch.hvm_vcpu.viridian.crash_param)); + + idx -= HV_X64_MSR_CRASH_P0; + *val = v->arch.hvm_vcpu.viridian.crash_param[idx]; + break; + + case HV_X64_MSR_CRASH_CTL: + { + HV_CRASH_CTL_REG_CONTENTS ctl = { + .CrashNotify = 1, + }; + + *val = ctl.AsUINT64; + break; + } + default: if ( idx >= VIRIDIAN_MSR_MIN && idx <= VIRIDIAN_MSR_MAX ) gprintk(XENLOG_WARNING, "read from unimplemented MSR %#x\n", diff --git a/xen/include/asm-x86/hvm/viridian.h b/xen/include/asm-x86/hvm/viridian.h index 271c36d..30259e9 100644 --- a/xen/include/asm-x86/hvm/viridian.h +++ b/xen/include/asm-x86/hvm/viridian.h @@ -26,6 +26,7 @@ struct viridian_vcpu void *va; int vector; } vp_assist; + uint64_t crash_param[5]; }; union viridian_guest_os_id diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h index 58c8478..19c9eb8 100644 --- a/xen/include/public/hvm/params.h +++ b/xen/include/public/hvm/params.h @@ -135,13 +135,18 @@ #define _HVMPV_apic_assist 5 #define HVMPV_apic_assist (1 << _HVMPV_apic_assist) +/* Enable crash MSRs */ +#define _HVMPV_crash_ctl 6 +#define HVMPV_crash_ctl (1 << _HVMPV_crash_ctl) + #define HVMPV_feature_mask \ (HVMPV_base_freq | \ HVMPV_no_freq | \ HVMPV_time_ref_count | \ HVMPV_reference_tsc | \ HVMPV_hcall_remote_tlb_flush | \ - HVMPV_apic_assist) + HVMPV_apic_assist | \ + HVMPV_crash_ctl) #endif