On 3/28/21 8:21 PM, Richard Henderson wrote: > On 3/26/21 1:36 PM, Claudio Fontana wrote: >> +++ b/target/arm/kvm/kvm-sve.h >> @@ -0,0 +1,30 @@ >> +/* >> + * QEMU AArch64 CPU SVE KVM interface >> + * >> + * Copyright 2021 SUSE LLC >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + */ >> + >> +#ifndef KVM_SVE_H >> +#define KVM_SVE_H >> + >> +/* note: SVE is an AARCH64-only option, only include this for >> TARGET_AARCH64 */ > > A pointless comment given that kvm itself is aarch64-only. > >> -void cpu_sve_finalize_features(ARMCPU *cpu, Error **errp) >> +bool cpu_sve_finalize_features(ARMCPU *cpu, Error **errp) > > The commit message does not mention the interface change here.
Ok, would need the two-steps here too. > > I'm not sure about the rest of this patch. It saves a miniscule amount of > code > in a kvm-only build, but I don't know that it clarifies things at all. I would say that it does, if only by making the code flow easier to follow. > > As yet, the other arm hw accelerators do not support SVE, but I assume that's > only a matter of time. The ARM Neoverse cpus support SVE, and will require > Xen > to have an answer soon. (Apple will do whatever Apple does, given that it > fabs > its own ARM cpus, but I expect they won't delay SVE forever.) > > It's not clear to me what bits of the kvm code here is really kvm specific, > or > if we'll have to move it back. > > I'm tempted to leave it alone for now. But fair enough, if nobody speaks up and finds them useful other than me, I would drop this from the series. > > > r~ >
