> 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.
While Andrew is working on the patch in a hurry, I'm sorry, I'll be on vacation for a while starting Friday too, so my reply will be delayed. Best regards. > -----Original Message----- > From: Andrew Jones <drjo...@redhat.com> > Sent: Wednesday, August 18, 2021 5:58 PM > To: Ishii, Shuuichirou/石井 周一郎 <ishii.shuuic...@fujitsu.com> > Cc: Richard Henderson <richard.hender...@linaro.org>; > peter.mayd...@linaro.org; qemu-...@nongnu.org; qemu-devel@nongnu.org > Subject: Re: [PATCH v4 1/3] target-arm: Add support for Fujitsu A64FX > > On Wed, Aug 18, 2021 at 08:29:15AM +0000, ishii.shuuic...@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 <drjo...@redhat.com> > > > Sent: Wednesday, August 18, 2021 1:28 AM > > > To: Richard Henderson <richard.hender...@linaro.org> > > > Cc: Ishii, Shuuichirou/石井 周一郎 <ishii.shuuic...@fujitsu.com>; > > > peter.mayd...@linaro.org; qemu-...@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 > >