[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 2/6] x86/viridian: don't put Xen version information in CPUID leaf 2
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@xxxxxxxx] > Sent: 22 March 2017 10:10 > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx> > Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; xen- > devel@xxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH v3 2/6] x86/viridian: don't put Xen version information in > CPUID leaf 2 > > >>> On 21.03.17 at 19:17, <paul.durrant@xxxxxxxxxx> wrote: > > --- a/docs/misc/xen-command-line.markdown > > +++ b/docs/misc/xen-command-line.markdown > > @@ -1616,6 +1616,14 @@ The optional `keep` parameter causes Xen to > continue using the vga > > console even after dom0 has been started. The default behaviour is to > > relinquish control to dom0. > > > > +### viridian-version > > +> `= <major>,<minor>,<build>` > > In my proposal I had intentionally enclosed each number in square > brackets, indicating that each one is optional. I don't see the point > of having to specify all numbers all the time (and your earlier split > option model didn't make this a requirement either). > So you'd like something like: viridian-version=,,0xabcd so that just the build number can be overridden to 0xabcd? > > +> Default: `6,0,1772` > > + > > +<major>, <minor> and <build> must be integers specified in hexadecimal. > The values will be > > +encoded in guest CPUID 0x40000002 if viridian enlightenments are > enabled. > > Please wrap at column 80 the latest. And why do the numbers need > to be in hex? I can relax that restriction if you'd prefer. > > > --- a/xen/arch/x86/hvm/viridian.c > > +++ b/xen/arch/x86/hvm/viridian.c > > @@ -106,6 +106,19 @@ typedef struct { > > #define CPUID6A_MSR_BITMAPS (1 << 1) > > #define CPUID6A_NESTED_PAGING (1 << 3) > > > > +/* > > + * Version and build number reported by CPUID leaf 2 > > + * > > + * These numbers are chosen to match the version numbers reported by > > + * Windows Server 2008. > > + */ > > +static char __initdata > viridian_version[sizeof("0xVVVV,0xVVVV,0xVVVVVVVV")] = ""; > > Pointless in initializer. > Seemed to be common practice elsewhere in the code. > > +string_param("viridian-version", viridian_version); > > Why not custom_param()? > Not come across custom_param(). I'll look it up. > > @@ -910,6 +923,39 @@ static int viridian_load_vcpu_ctxt(struct domain > *d, hvm_domain_context_t *h) > > HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_VCPU, > viridian_save_vcpu_ctxt, > > viridian_load_vcpu_ctxt, 1, HVMSR_PER_VCPU); > > > > +static int __init viridian_init(void) > > +{ > > + char *t, *v = viridian_version; > > With e being pointer to const, I think t should be, too. > Ok. > > + unsigned int n[3], i = 0; > > + > > + if ( *v == '\0' ) > > + return 0; > > + > > + while ( (t = strsep(&v, ",")) != NULL ) > > + { > > + const char *e; > > + > > + n[i++] = simple_strtoul(t, &e, 16); > > + if ( *e != '\0' ) > > + goto fail; > > + } > > + if ( i != 3 ) > > + goto fail; > > + > > + if (n[0] > 0xffff || n[1] > 0xffff) > > Missing blanks. > Oops. > > + goto fail; > > + > > + viridian_major = n[0]; > > + viridian_minor = n[1]; > > + viridian_build = n[2]; > > + return 0; > > + > > +fail: > > Indentation. > Yes, sorry I forgot about that xen style quirk. Paul > Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |