On Sun, 2019-06-09 at 09:20 +0200, Maxime Villard wrote:
> Le 07/06/2019 à 20:33, Michał Górny a écrit :
> > +#define XSAVE_MAX_COMPONENT XSAVE_Hi16_ZMM
> > [...]
> > +size_t x86_xsave_offsets[XSAVE_MAX_COMPONENT] __read_mostly;
> > +size_t x86_xsave_sizes[XSAVE_MAX_COMPONENT] __read_mostly;
>
> Need +1, but see below
Right.
>
> > + /* 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)) {
>
> I would use __BIT(i).
Makes sense.
>
> In fact, the whole loop seems wrong: CPUID uses the XCR0_ constants, not the
> XSAVE_ constants. Eg HDC is 13 in XCR0_ but 10 in XSAVE_, so you never iterate
> over it.
It's intentional. I only iterate up to XSAVE_MAX_COMPONENT,
i.e. the last component actually used by the kernel. I don't skip
earlier unused components to avoid index madness but I see no purpose to
go beyond last component actually used.
Of course, if you prefer it that way, I can extend the array and go over
all of them.
>
> I would drop XSAVE_* entirely and just use XCR0_*, it is forward compatible.
Unless I'm mistaken, that would require using log2() to get array
indices.
>
> > -#define __NetBSD_Version__ 899004200 /* NetBSD 8.99.42 */
> > +#define __NetBSD_Version__ 899004300 /* NetBSD 8.99.43 */
>
> No version bump needed for that
Ok.
--
Best regards,
Michał Górny
Attachment:
signature.asc
Description: This is a digitally signed message part