linux-kernel - Re: [PATCH v2] arm64/fpsimd: Only provide the length to cpufeature for xCR registers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZM0lRWTAE/b88V6U@arm.com>
Date:   Fri, 4 Aug 2023 17:20:21 +0100
From:   Catalin Marinas <catalin.marinas@....com>
To:     Mark Brown <broonie@...nel.org>
Cc:     Will Deacon <will@...nel.org>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] arm64/fpsimd: Only provide the length to cpufeature
 for xCR registers

On Thu, Aug 03, 2023 at 06:44:24PM +0100, Mark Brown wrote:
> On Thu, Aug 03, 2023 at 05:39:38PM +0100, Catalin Marinas wrote:
> > On Mon, Jul 31, 2023 at 02:58:48PM +0100, Mark Brown wrote:
> > > Since the only field we are interested in having the cpufeature code
> > > handle is the length field and we use a custom read function to obtain
> > > the value we can avoid these warnings by filtering out all other bits
> > > when we return the register value, if we're doing that we don't need to
> > > bother reading the register at all and can simply use the RDVL/RDSVL
> > > value we were filling in instead.
> 
> > Maybe that's the simplest fix, especially if you want it in stable, but
> 
> Yeah, it's definitely the sort of change we want as a fix - anything
> more invasive would be inappropriate.

I'd say it's still ok if we can just rip come code out safely (the fake
ID reg).

> > I wonder why we even bother with with treating ZCR_EL1 and SMCR_EL1 as
> > feature registers. We already have verify_sme_features() to check for
> > the mismatch. BTW, is vec_verify_vq_map() sufficient so that we can skip
> > the maximum vector length check?
> 
> Both enumeration mechanisms were added in the initial series supporting
> SVE for reasons that are not entirely obvious to me.  The changelogs
> explain what we're doing with the pseudo ID register stuff but do not
> comment on why.  There is a cross check between the answers the two give
> which appears to be geared towards detecting systems with asymmetric
> maximum VLs for some reason but I'm not sure why that's done given that
> we can't cope if *any* VL in the committed set is missing, not just the
> maximum.

We can cope with different VLs if the committed map is built during boot
(early secondary CPU bring-up). For any late/hotplugged CPUs, if they
don't fit the map, they'll be rejected. Not sure where the actual
maximum length matters in this process though (or later for user space).
I assume the user will only be allowed to set the common VLs across all
the early CPUs.

> The whole thing is very suspect but given that we don't currently have
> any ability to emulate systems with asymmetric vector lengths I'm a bit
> reluctant to poke at it.

The Arm fast models should allow such configuration, though I haven't
tried.

-- 
Catalin