On 3/25/21 7:48 PM, Claudio Fontana wrote: > On 3/25/21 7:40 PM, Richard Henderson wrote: >> On 3/23/21 9:46 AM, Claudio Fontana wrote: >>> extract the SVE-related cpu object properties and functions, >>> and move them to a separate module. >>> >>> Disentangle SVE from pauth that is a separate, TCG-only feature. >>> >>> Signed-off-by: Claudio Fontana <cfont...@suse.de> >>> --- >>> target/arm/cpu-sve.h | 40 +++++ >>> target/arm/cpu.h | 22 +-- >>> target/arm/cpu-sve.c | 360 +++++++++++++++++++++++++++++++++++++++ >>> target/arm/cpu.c | 5 +- >>> target/arm/cpu64.c | 333 +----------------------------------- >>> target/arm/kvm/kvm-cpu.c | 2 +- >>> target/arm/meson.build | 1 + >>> 7 files changed, 417 insertions(+), 346 deletions(-) >>> create mode 100644 target/arm/cpu-sve.h >>> create mode 100644 target/arm/cpu-sve.c >>> >>> diff --git a/target/arm/cpu-sve.h b/target/arm/cpu-sve.h >>> new file mode 100644 >>> index 0000000000..b1be575265 >>> --- /dev/null >>> +++ b/target/arm/cpu-sve.h >>> @@ -0,0 +1,40 @@ >>> +/* >>> + * QEMU AArch64 CPU SVE Extensions for TARGET_AARCH64 >>> + * >>> + * Copyright (c) 2013 Linaro Ltd >>> + * >>> + * This program is free software; you can redistribute it and/or >>> + * modify it under the terms of the GNU General Public License >>> + * as published by the Free Software Foundation; either version 2 >>> + * of the License, or (at your option) any later version. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + * GNU General Public License for more details. >>> + * >>> + * You should have received a copy of the GNU General Public License >>> + * along with this program; if not, see >>> + * <http://www.gnu.org/licenses/gpl-2.0.html> >>> + */ >>> + >>> +#ifndef CPU_SVE_H >>> +#define CPU_SVE_H >>> + >>> +/* note: SVE is an AARCH64-only option, only include this for >>> TARGET_AARCH64 */ >>> + >>> +/* called by arm_cpu_finalize_features in realizefn */ >>> +void cpu_sve_finalize_features(ARMCPU *cpu, Error **errp); >>> + >>> +/* add the CPU SVE properties */ >>> +void cpu_sve_add_props(Object *obj); >>> + >>> +/* add the CPU SVE properties specific to the "MAX" CPU */ >>> +void cpu_sve_add_props_max(Object *obj); >>> + >>> +/* In AArch32 mode, predicate registers do not exist at all. */ >>> +typedef struct ARMPredicateReg { >>> + uint64_t p[DIV_ROUND_UP(2 * ARM_MAX_VQ, 8)] QEMU_ALIGNED(16); >>> +} ARMPredicateReg; >> >> I don't agree with moving this out of cpu.h, next to the rest of the vector >> definitions. >> >> I agree that we should be using more files, but if we're going to have an >> cpu-sve.c, why did some of the sve functions go into cpu-common.c?
actually, now that the option of making those SVE functions in cpu-common.c TARGET_AARCH64-specific is open, we could attempt to import them in cpu-sve. I'll give it a try, lets see the result. > > maybe cpu-sve-props.c would be a better name, it really contains only cpu sve > properties code. > >> >> I don't agree with moving functions and renaming them simultaneously. I'm >> not >> even sure why you're renaming them, or why you've suddenly chosen >> "cpu_sve_*" >> as the prefix to use. >> >> >> r~ >> > > I think the idea was trying to create a cpu_sve module handling everything > related to sve, > and consistently using the name of the module as the prefix. > > It might be too early to attempt that anyway; as you noted, there is other > SVE-related functionality all over the place, > so better to revisit this. > > I'd suggest renaming this to cpu_sve_props, and moving everything not props > related out of it. > > Thanks, > > Claudio > > > >
