Re: [PATCH v4 1/3] target-arm: Add support for Fujitsu A64FX

[Andrew Jones]

On Wed, Aug 18, 2021 at 08:29:15AM +0000, ishii.shuuichir@fujitsu.com wrote:
> 
> We appreciate everyone's comments.
> Before making the V5 patch, please let me check the patch contents.
> 
> > This looks reasonable to me, but you also need the 'sve' property that 
> > states sve in
> > supported at all.
> > > > So maybe we should just go ahead and add all sve* properties, 
> 
> In response to the above comment,
> We understood that the sve property will be added to the v4 patch.
> 
> i.e. 
> (QEMU) query-cpu-model-expansion type=full model={"name":"a64fx"}
> {"return": {"model": {"name": "a64fx", "props": {"sve128": false, "sve256": 
> true, "sve": true, "sve512": true, "aarch64": true, "pmu": true}}}}
> 
> > > > but
> > > > then make sure the default vq map is correct.
> 
> Furthermore, We understood that I need to add the above process as well, is 
> that correct?
> 
> > That's a good idea. I'll send a patch with your suggested-by.
> 
> If that's correct,
> In the current v4 patch, in the aarch64_a64fx_initfn function, 
> the a64fx_cpu_set_sve function is executed to set the SVE property, 
> and the arm_cpu_sve_finalize function is not called. 
> 
> In which function is it appropriate to execute the modulo max_vq function 
> (or equivalent process)?
> 
> If We are not understanding you correctly,
> We would appreciate your comments.

Richard's suggestion is to generalize the "supported" bitmap concept,
which is currently only used for KVM, in order to also use it for
TCG cpu models. The 'max' cpu type will have the trivial all-set
supported bitmap, but the a64fx will have a specific one. I plan to
do this "supported" bitmap generalization and apply it to the TCG
max cpu type. You'll need to rebase this series on those patches and
provide the a64fx supported bitmap.

I think this will be more clear once I get the patch posted (which I
haven't started writing yet). I'll try to get it posted by tomorrow
evening though, since I have vacation on Friday.

Thanks,
drew


> 
> Best regards.
> 
> > -----Original Message-----
> > From: Andrew Jones <drjones@redhat.com>
> > Sent: Wednesday, August 18, 2021 1:28 AM
> > To: Richard Henderson <richard.henderson@linaro.org>
> > Cc: Ishii, Shuuichirou/石井 周一郎 <ishii.shuuichir@fujitsu.com>;
> > peter.maydell@linaro.org; qemu-arm@nongnu.org; qemu-devel@nongnu.org
> > Subject: Re: [PATCH v4 1/3] target-arm: Add support for Fujitsu A64FX
> > 
> > On Tue, Aug 17, 2021 at 05:53:34AM -1000, Richard Henderson wrote:
> > > On 8/17/21 5:36 AM, Andrew Jones wrote:
> > > > On Tue, Aug 17, 2021 at 05:23:17AM -1000, Richard Henderson wrote:
> > > > > On 8/17/21 1:56 AM, Andrew Jones wrote:
> > > > > > I guess it's fine. You could easily create a new
> > > > > > cpu_arm_set_sve_vq() which would forbid changing the properties
> > > > > > if you wanted to, but then we need to answer Peter's question in
> > > > > > order to see if there's a precedent for that type of property.
> > > > >
> > > > > I don't see the point in read-only properties.  If the user wants
> > > > > to set non-standard values on the command-line, let them.  What is
> > > > > most important is getting the correct default from '-cpu a64fx'.
> > > > >
> > > >
> > > > So maybe we should just go ahead and add all sve* properties, but
> > > > then make sure the default vq map is correct.
> > >
> > > I think that's the right answer.
> > >
> > > Presently we have a kvm_supported variable that's initialized by
> > > kvm_arm_sve_get_vls().  I think we want to rename that variable and
> > > provide a version of that function for tcg. Probably we should have
> > > done that before, with a trivial function for -cpu max to set all bits.
> > >
> > > Then eliminate most of the other kvm_enabled() checks in
> > > arm_cpu_sve_finalize.  I think the only one we keep is the last, where
> > > we verify that the final sve_vq_map matches kvm_enabled exactly, modulo
> > max_vq.
> > >
> > > This should minimize the differences in behaviour between tcg and kvm.
> > 
> > That's a good idea. I'll send a patch with your suggested-by.
> > 
> > Thanks,
> > drew
>