Re: [PATCH v2 1/2] Fetch XSAVE area component offsets and sizes when initializing x86 CPU

[Paul Goyette <paul%whooppee.com@localhost>]

On Wed, 5 Jun 2019, Kamil Rytarowski wrote:

> On 05.06.2019 17:32, Paul Goyette wrote:
> > Are there any userland programs that use cpu.h?  Or any kernel modules?
>
> We use cpu.h, e.g. when prompting for CPU_MACHDEP values with sysctl(3).
>
> HAXM as a kernel module uses cpu.h.

Well, just #include doesn't mean we need a bump.  Do either of these 
result in a change to the data being shared with the kernel?

For example, does HAXM actually share any 'struct x86_fpu_save' (or any 
larger struct that contains a x86_fpu_save) with the kernel?  Or do the 
sysctl variables include a struct x86_fpu_save?



> > If so, these changes would create a change in the kernel API and would
> > therefore require a kernel version bump.
> >
> >
> >
> >
> > > Introduce two new arrays, x86_xsave_offsets and x86_xsave_sizes,
> > > and initialize them with XSAVE area component offsets and sizes queried
> > > via CPUID.  This will be needed to implement getters and setters for
> > > additional register types.
> > >
> > > While at it, add XSAVE_* constants corresponding to specific XSAVE
> > > components.
> > > ---
> > > sys/arch/x86/include/cpu.h        |  2 ++
> > > sys/arch/x86/include/specialreg.h | 20 ++++++++++++++++++++
> > > sys/arch/x86/x86/identcpu.c       | 12 ++++++++++++
> > > 3 files changed, 34 insertions(+)
> > >
> > > diff --git a/sys/arch/x86/include/cpu.h b/sys/arch/x86/include/cpu.h
> > > index 143ae3c5c5ec..589f179ce758 100644
> > > --- a/sys/arch/x86/include/cpu.h
> > > +++ b/sys/arch/x86/include/cpu.h
> > > @@ -459,6 +459,8 @@ extern int x86_fpu_save;
> > > #define    FPU_SAVE_XSAVEOPT    3
> > > extern unsigned int x86_fpu_save_size;
> > > extern uint64_t x86_xsave_features;
> > > +extern size_t x86_xsave_offsets[];
> > > +extern size_t x86_xsave_sizes[];
> > > extern uint32_t x86_fpu_mxcsr_mask;
> > > extern bool x86_fpu_eager;
> > >
> > > diff --git a/sys/arch/x86/include/specialreg.h
> > > b/sys/arch/x86/include/specialreg.h
> > > index 4f8c4cca6db7..1c0e8c972b07 100644
> > > --- a/sys/arch/x86/include/specialreg.h
> > > +++ b/sys/arch/x86/include/specialreg.h
> > > @@ -146,6 +146,26 @@
> > > #define XCR0_FPU    (XCR0_X87 | XCR0_SSE | XCR0_YMM_Hi128 | \
> > >              XCR0_Opmask | XCR0_ZMM_Hi256 | XCR0_Hi16_ZMM)
> > >
> > > +/*
> > > + * XSAVE component indices.
> > > + */
> > > +#define XSAVE_X87    0
> > > +#define XSAVE_SSE    1
> > > +#define XSAVE_YMM_Hi128    2
> > > +#define XSAVE_BNDREGS    3
> > > +#define XSAVE_BNDCSR    4
> > > +#define XSAVE_Opmask    5
> > > +#define XSAVE_ZMM_Hi256    6
> > > +#define XSAVE_Hi16_ZMM    7
> > > +#define XSAVE_PT    8
> > > +#define XSAVE_PKRU    9
> > > +#define XSAVE_HDC    10
> > > +
> > > +/*
> > > + * Highest XSAVE component enabled by XCR0_FPU.
> > > + */
> > > +#define XSAVE_MAX_COMPONENT XSAVE_Hi16_ZMM
> > > +
> > > /*
> > >  * CPUID "features" bits
> > >  */
> > > diff --git a/sys/arch/x86/x86/identcpu.c b/sys/arch/x86/x86/identcpu.c
> > > index 9037fb2673fd..9714865bfb43 100644
> > > --- a/sys/arch/x86/x86/identcpu.c
> > > +++ b/sys/arch/x86/x86/identcpu.c
> > > @@ -74,6 +74,8 @@ char cpu_brand_string[49];
> > > int x86_fpu_save __read_mostly;
> > > unsigned int x86_fpu_save_size __read_mostly = sizeof(struct save87);
> > > uint64_t x86_xsave_features __read_mostly = 0;
> > > +size_t x86_xsave_offsets[XSAVE_MAX_COMPONENT] __read_mostly;
> > > +size_t x86_xsave_sizes[XSAVE_MAX_COMPONENT] __read_mostly;
> > >
> > > /*
> > >  * Note: these are just the ones that may not have a cpuid instruction.
> > > @@ -755,6 +757,7 @@ static void
> > > cpu_probe_fpu(struct cpu_info *ci)
> > > {
> > >     u_int descs[4];
> > > +    int i;
> > >
> > >     x86_fpu_eager = true;
> > >     x86_fpu_save = FPU_SAVE_FSAVE;
> > > @@ -816,6 +819,15 @@ cpu_probe_fpu(struct cpu_info *ci)
> > >         x86_fpu_save_size = descs[2];
> > >
> > >     x86_xsave_features = (uint64_t)descs[3] << 32 | descs[0];
> > > +
> > > +    /* Get component offsets and sizes for the save area */
> > > +    for (i = XSAVE_YMM_Hi128; i < __arraycount(x86_xsave_offsets);
> > > i++) {
> > > +        if (x86_xsave_features & ((uint64_t)1 << i)) {
> > > +            x86_cpuid2(0xd, i, descs);
> > > +            x86_xsave_offsets[i] = descs[1];
> > > +            x86_xsave_sizes[i] = descs[0];
> > > +        }
> > > +    }
> > > }
> > >
> > > void
> > > -- 
> > > 2.22.0.rc3
> > >
> > >
> > > !DSPAM:5cf7db2f272681658520559!
> >
> > +--------------------+--------------------------+-----------------------+
> > | Paul Goyette       | PGP Key fingerprint:     | E-mail addresses:     |
> > | (Retired)          | FA29 0E3B 35AF E8AE 6651 | paul%whooppee.com@localhost     |
> > | Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette%netbsd.org@localhost   |
> > +--------------------+--------------------------+-----------------------+

+--------------------+--------------------------+-----------------------+
| Paul Goyette       | PGP Key fingerprint:     | E-mail addresses:     |
| (Retired)          | FA29 0E3B 35AF E8AE 6651 | paul%whooppee.com@localhost     |
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette%netbsd.org@localhost   |
+--------------------+--------------------------+-----------------------+