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

[address@hidden]

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.

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