[PATCH] cpu_map: Introduce A64FX

Liu Yiding posted 1 patch 1 year, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1661937736-2-1-git-send-email-liuyd.fnst@fujitsu.com
src/cpu_map/arm_A64FX.xml   | 6 ++++++
src/cpu_map/arm_vendors.xml | 1 +
src/cpu_map/index.xml       | 3 +++
src/cpu_map/meson.build     | 1 +
4 files changed, 11 insertions(+)
create mode 100644 src/cpu_map/arm_A64FX.xml
[PATCH] cpu_map: Introduce A64FX
Posted by Liu Yiding 1 year, 8 months ago
Add A64FX as a supported cpu model.

Signed-off-by: Liu Yiding <liuyd.fnst@fujitsu.com>
---
 src/cpu_map/arm_A64FX.xml   | 6 ++++++
 src/cpu_map/arm_vendors.xml | 1 +
 src/cpu_map/index.xml       | 3 +++
 src/cpu_map/meson.build     | 1 +
 4 files changed, 11 insertions(+)
 create mode 100644 src/cpu_map/arm_A64FX.xml

diff --git a/src/cpu_map/arm_A64FX.xml b/src/cpu_map/arm_A64FX.xml
new file mode 100644
index 0000000000..db7b328cd1
--- /dev/null
+++ b/src/cpu_map/arm_A64FX.xml
@@ -0,0 +1,6 @@
+<cpus>
+  <model name='a64fx'>
+    <vendor name='FUJITSU'/>
+    <pvr value='0x001'/>
+  </model>
+</cpus>
diff --git a/src/cpu_map/arm_vendors.xml b/src/cpu_map/arm_vendors.xml
index 4465463b5b..b10c07815a 100644
--- a/src/cpu_map/arm_vendors.xml
+++ b/src/cpu_map/arm_vendors.xml
@@ -3,6 +3,7 @@
   <vendor name='Broadcom' value='0x42'/>
   <vendor name='Cavium' value='0x43'/>
   <vendor name='DigitalEquipment' value='0x44'/>
+  <vendor name='FUJITSU' value='0x46'/>
   <vendor name='HiSilicon' value='0x48'/>
   <vendor name='Infineon' value='0x49'/>
   <vendor name='Freescale' value='0x4D'/>
diff --git a/src/cpu_map/index.xml b/src/cpu_map/index.xml
index 351c2ae4fa..44534f067a 100644
--- a/src/cpu_map/index.xml
+++ b/src/cpu_map/index.xml
@@ -104,6 +104,9 @@
     <!-- Cavium-based CPU models -->
     <include filename='arm_ThunderX299xx.xml'/>
 
+    <!-- Fujitsu-based CPU models -->
+    <include filename='arm_A64FX.xml'/>
+
     <!-- Hisilicon-based CPU models -->
     <include filename='arm_Kunpeng-920.xml'/>
 
diff --git a/src/cpu_map/meson.build b/src/cpu_map/meson.build
index 99815981b5..af7e5bc9f3 100644
--- a/src/cpu_map/meson.build
+++ b/src/cpu_map/meson.build
@@ -1,4 +1,5 @@
 cpumap_data = [
+  'arm_A64FX.xml',
   'arm_cortex-a53.xml',
   'arm_cortex-a57.xml',
   'arm_cortex-a72.xml',
-- 
2.34.1
Re: [PATCH] cpu_map: Introduce A64FX
Posted by Jiri Denemark 1 year, 8 months ago
On Wed, Aug 31, 2022 at 09:22:16 +0000, Liu Yiding wrote:
> Add A64FX as a supported cpu model.
> 
> Signed-off-by: Liu Yiding <liuyd.fnst@fujitsu.com>
> ---
>  src/cpu_map/arm_A64FX.xml   | 6 ++++++
>  src/cpu_map/arm_vendors.xml | 1 +
>  src/cpu_map/index.xml       | 3 +++
>  src/cpu_map/meson.build     | 1 +
>  4 files changed, 11 insertions(+)
>  create mode 100644 src/cpu_map/arm_A64FX.xml
> 
> diff --git a/src/cpu_map/arm_A64FX.xml b/src/cpu_map/arm_A64FX.xml
> new file mode 100644
> index 0000000000..db7b328cd1
> --- /dev/null
> +++ b/src/cpu_map/arm_A64FX.xml
> @@ -0,0 +1,6 @@
> +<cpus>
> +  <model name='a64fx'>

The model name is "a64fx" while the file name contains A64FX. We should
be consistent and use the same spelling for both.

> +    <vendor name='FUJITSU'/>
> +    <pvr value='0x001'/>
> +  </model>
> +</cpus>
> diff --git a/src/cpu_map/arm_vendors.xml b/src/cpu_map/arm_vendors.xml
> index 4465463b5b..b10c07815a 100644
> --- a/src/cpu_map/arm_vendors.xml
> +++ b/src/cpu_map/arm_vendors.xml
> @@ -3,6 +3,7 @@
>    <vendor name='Broadcom' value='0x42'/>
>    <vendor name='Cavium' value='0x43'/>
>    <vendor name='DigitalEquipment' value='0x44'/>
> +  <vendor name='FUJITSU' value='0x46'/>

You spell the vendor name as "Fujitsu" below in the cpu_map comment.
Which one is correct?

>    <vendor name='HiSilicon' value='0x48'/>
>    <vendor name='Infineon' value='0x49'/>
>    <vendor name='Freescale' value='0x4D'/>
> diff --git a/src/cpu_map/index.xml b/src/cpu_map/index.xml
> index 351c2ae4fa..44534f067a 100644
> --- a/src/cpu_map/index.xml
> +++ b/src/cpu_map/index.xml
> @@ -104,6 +104,9 @@
>      <!-- Cavium-based CPU models -->
>      <include filename='arm_ThunderX299xx.xml'/>
>  
> +    <!-- Fujitsu-based CPU models -->
> +    <include filename='arm_A64FX.xml'/>
> +
>      <!-- Hisilicon-based CPU models -->
>      <include filename='arm_Kunpeng-920.xml'/>
>  
> diff --git a/src/cpu_map/meson.build b/src/cpu_map/meson.build
> index 99815981b5..af7e5bc9f3 100644
> --- a/src/cpu_map/meson.build
> +++ b/src/cpu_map/meson.build
> @@ -1,4 +1,5 @@
>  cpumap_data = [
> +  'arm_A64FX.xml',
>    'arm_cortex-a53.xml',
>    'arm_cortex-a57.xml',
>    'arm_cortex-a72.xml',

Jirka
Re: [PATCH] cpu_map: Introduce A64FX
Posted by Jiri Denemark 1 year, 8 months ago
On Wed, Aug 31, 2022 at 09:22:16 +0000, Liu Yiding wrote:
> Add A64FX as a supported cpu model.
> 
> Signed-off-by: Liu Yiding <liuyd.fnst@fujitsu.com>
> ---
>  src/cpu_map/arm_A64FX.xml   | 6 ++++++
>  src/cpu_map/arm_vendors.xml | 1 +
>  src/cpu_map/index.xml       | 3 +++
>  src/cpu_map/meson.build     | 1 +
>  4 files changed, 11 insertions(+)
>  create mode 100644 src/cpu_map/arm_A64FX.xml

CPU detection for ARM is a bit awkward and not very useful in practice.
It will (in some cases) report the host CPU in host capabilities, but
that's it. The CPU from host capabilities cannot really be used in
general for specifying a guest CPU in domain XML and it would not even
make a lot of sense as I believe the only really supported CPU for KVM
is "host" (mode="host-passthrough" in domain XML).

I'm not saying the patch is wrong, I'm just trying to make sure you
don't expect something more from implementing this patch.

Jirka
Re: [PATCH] cpu_map: Introduce A64FX
Posted by Liu Yiding 1 year, 8 months ago
Hi Jiri

On 8/31/22 20:30, Jiri Denemark wrote:
> On Wed, Aug 31, 2022 at 09:22:16 +0000, Liu Yiding wrote:
>> Add A64FX as a supported cpu model.
>>
>> Signed-off-by: Liu Yiding <liuyd.fnst@fujitsu.com>
>> ---
>>   src/cpu_map/arm_A64FX.xml   | 6 ++++++
>>   src/cpu_map/arm_vendors.xml | 1 +
>>   src/cpu_map/index.xml       | 3 +++
>>   src/cpu_map/meson.build     | 1 +
>>   4 files changed, 11 insertions(+)
>>   create mode 100644 src/cpu_map/arm_A64FX.xml
> 
> CPU detection for ARM is a bit awkward and not very useful in practice.
> It will (in some cases) report the host CPU in host capabilities, but
> that's it. The CPU from host capabilities cannot really be used in
> general for specifying a guest CPU in domain XML and it would not even
> make a lot of sense as I believe the only really supported CPU for KVM
> is "host" (mode="host-passthrough" in domain XML).
> 

Thanks for your detailed explanation. :)

> I'm not saying the patch is wrong, I'm just trying to make sure you
> don't expect something more from implementing this patch.
> 

Yes, I just want to fix the err logging. We have a tp-libvirt case that 
always fails with this err logging...

As your explanations, It's ok for me to ignore this patch as it does a 
little effect. :)

Thanks again.

> Jirka
> 

-- 
Thanks,
Yiding
Re: [PATCH] cpu_map: Introduce A64FX
Posted by Jiri Denemark 1 year, 8 months ago
On Thu, Sep 01, 2022 at 09:43:56 +0800, Liu Yiding wrote:
> Hi Jiri
> 
> On 8/31/22 20:30, Jiri Denemark wrote:
> > On Wed, Aug 31, 2022 at 09:22:16 +0000, Liu Yiding wrote:
> >> Add A64FX as a supported cpu model.
> >>
> >> Signed-off-by: Liu Yiding <liuyd.fnst@fujitsu.com>
> >> ---
> >>   src/cpu_map/arm_A64FX.xml   | 6 ++++++
> >>   src/cpu_map/arm_vendors.xml | 1 +
> >>   src/cpu_map/index.xml       | 3 +++
> >>   src/cpu_map/meson.build     | 1 +
> >>   4 files changed, 11 insertions(+)
> >>   create mode 100644 src/cpu_map/arm_A64FX.xml
> > 
> > CPU detection for ARM is a bit awkward and not very useful in practice.
> > It will (in some cases) report the host CPU in host capabilities, but
> > that's it. The CPU from host capabilities cannot really be used in
> > general for specifying a guest CPU in domain XML and it would not even
> > make a lot of sense as I believe the only really supported CPU for KVM
> > is "host" (mode="host-passthrough" in domain XML).
> > 
> 
> Thanks for your detailed explanation. :)
> 
> > I'm not saying the patch is wrong, I'm just trying to make sure you
> > don't expect something more from implementing this patch.
> > 
> 
> Yes, I just want to fix the err logging. We have a tp-libvirt case that 
> always fails with this err logging...
> 
> As your explanations, It's ok for me to ignore this patch as it does a 
> little effect. :)

There's no reason for ignoring the patch completely. We support
detecting CPUs from other vendors as they sent patches so why should
Fujitsu be different.

Jirka
Re: [PATCH] cpu_map: Introduce A64FX
Posted by Daniel P. Berrangé 1 year, 8 months ago
On Wed, Aug 31, 2022 at 02:30:52PM +0200, Jiri Denemark wrote:
> On Wed, Aug 31, 2022 at 09:22:16 +0000, Liu Yiding wrote:
> > Add A64FX as a supported cpu model.
> > 
> > Signed-off-by: Liu Yiding <liuyd.fnst@fujitsu.com>
> > ---
> >  src/cpu_map/arm_A64FX.xml   | 6 ++++++
> >  src/cpu_map/arm_vendors.xml | 1 +
> >  src/cpu_map/index.xml       | 3 +++
> >  src/cpu_map/meson.build     | 1 +
> >  4 files changed, 11 insertions(+)
> >  create mode 100644 src/cpu_map/arm_A64FX.xml
> 
> CPU detection for ARM is a bit awkward and not very useful in practice.
> It will (in some cases) report the host CPU in host capabilities, but
> that's it. The CPU from host capabilities cannot really be used in
> general for specifying a guest CPU in domain XML and it would not even
> make a lot of sense as I believe the only really supported CPU for KVM
> is "host" (mode="host-passthrough" in domain XML).

This still lets it be used for a guest with TCG, which has no requirement
to use 'host'.

> I'm not saying the patch is wrong, I'm just trying to make sure you
> don't expect something more from implementing this patch.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH] cpu_map: Introduce A64FX
Posted by Jiri Denemark 1 year, 8 months ago
On Wed, Aug 31, 2022 at 15:11:41 +0100, Daniel P. Berrangé wrote:
> On Wed, Aug 31, 2022 at 02:30:52PM +0200, Jiri Denemark wrote:
> > On Wed, Aug 31, 2022 at 09:22:16 +0000, Liu Yiding wrote:
> > > Add A64FX as a supported cpu model.
> > > 
> > > Signed-off-by: Liu Yiding <liuyd.fnst@fujitsu.com>
> > > ---
> > >  src/cpu_map/arm_A64FX.xml   | 6 ++++++
> > >  src/cpu_map/arm_vendors.xml | 1 +
> > >  src/cpu_map/index.xml       | 3 +++
> > >  src/cpu_map/meson.build     | 1 +
> > >  4 files changed, 11 insertions(+)
> > >  create mode 100644 src/cpu_map/arm_A64FX.xml
> > 
> > CPU detection for ARM is a bit awkward and not very useful in practice.
> > It will (in some cases) report the host CPU in host capabilities, but
> > that's it. The CPU from host capabilities cannot really be used in
> > general for specifying a guest CPU in domain XML and it would not even
> > make a lot of sense as I believe the only really supported CPU for KVM
> > is "host" (mode="host-passthrough" in domain XML).
> 
> This still lets it be used for a guest with TCG, which has no requirement
> to use 'host'.

The host CPU is irrelevant for TCG and libvirt on ARM has always allowed
any CPU model to be used, even those that are not in cpu_map. And since
a lot of the ARM CPUs in cpu_map do not match anything in QEMU anyway,
which is the reason why host CPU detection on ARM is mostly a cosmetic
thing.

> > I'm not saying the patch is wrong, I'm just trying to make sure you
> > don't expect something more from implementing this patch.

Jirka