Bug 765804 - Add CPUID support for VIA CPU
Summary: Add CPUID support for VIA CPU
Keywords:
Status: CLOSED DEFERRED
Alias: None
Product: Virtualization Tools
Classification: Community
Component: libvirt
Version: unspecified
Hardware: other
OS: All
unspecified
unspecified
Target Milestone: ---
Assignee: Libvirt Maintainers
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-12-09 13:40 UTC by Nico Prenzel
Modified: 2016-04-26 19:42 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-03-23 14:57:52 UTC


Attachments (Terms of Use)

Description Nico Prenzel 2011-12-09 13:40:19 UTC
Description of problem:
The following patch introduced CPUID support for VIA CPU into KVM to passthrough VIA-specific features to the KVM guest:
http://git.kernel.org/?p=virt/kvm/kvm.git;a=commit;h=4429d5dc1197aaf8188e4febcde54d26a51baf6c

Please add the possibility to add these CPUID features to a KVM guest through libvirt.

Version-Release number of selected component (if applicable):
0.9.8

How reproducible:
Feature not implemented

Steps to Reproduce:
1.
2.
3.
  
Actual results:
VIA CPUID's can't be passed to KVM guests

Expected results:
KVM guests can use the specific VIA CPU features configure with libvirt.

Additional info:

Comment 1 Dave Allan 2011-12-12 19:54:56 UTC
Hi Nico,

I don't have access to the appropriate hardware to implement this code.  Would you be willing to submit a patch?  I'm fairly certain that it's not difficult to do so, but it's been a while since I've been deeply in the code, so I'm not 100% certain.

Dave

Comment 2 Nico Prenzel 2011-12-13 08:34:08 UTC
Hi Dave,

Ok, I'd like to create such a patch. But I've not succeeded the first time I tried to get these feature's passed through. I've too, one of these VIA EPIA M850 wich has one of these Via Nano CPU's. So it should be possible to test it.

What would be my starting point?
The last time I tried to start with the configuration
http://berrange.com/posts/2010/02/15/guest-cpu-model-configuration-in-libvirt-with-qemukvm/
and went down to the cpu_map.xml... but i've not finished and i don't know what to do to extend libvirt to pass these features through.

The features needed to passthorugh:
/* VIA/Cyrix/Centaur-defined CPU features, CPUID level 0xC0000001, word 5 */
#define X86_FEATURE_XSTORE      (5*32+ 2) /* on-CPU RNG present (xstore insn) */
#define X86_FEATURE_XSTORE_EN   (5*32+ 3) /* on-CPU RNG enabled */
#define X86_FEATURE_XCRYPT      (5*32+ 6) /* on-CPU crypto (xcrypt insn) */
#define X86_FEATURE_XCRYPT_EN   (5*32+ 7) /* on-CPU crypto enabled */
#define X86_FEATURE_ACE2        (5*32+ 8) /* Advanced Cryptography Engine v2 */
#define X86_FEATURE_ACE2_EN     (5*32+ 9) /* ACE v2 enabled */
#define X86_FEATURE_PHE         (5*32+ 10) /* PadLock Hash Engine */
#define X86_FEATURE_PHE_EN      (5*32+ 11) /* PHE enabled */
#define X86_FEATURE_PMM         (5*32+ 12) /* PadLock Montgomery Multiplier */
#define X86_FEATURE_PMM_EN      (5*32+ 13) /* PMM enabled */

Thanks.

Comment 3 Dave Allan 2011-12-13 15:53:04 UTC
Jiri, you're more familiar with this area of the code than I am, cna you provide any guidance?

Comment 4 Jiri Denemark 2011-12-14 14:26:35 UTC
Hi Nico,

Ideally the only thing needed is the expansion of the cpu_map.xml file you
already mentioned. First, you need to add a new version string (this is
vendor_id value in /proc/cpuinfo):

    <vendor name='VIA' string='...'/>

Next, you need to define the CPU features QEMU understands:

    <feature name='...'>
      <cpuid function='0xc0000001' e?x='...'/>
    </feature>

where feature name should correspond to the name qemu accepts in
-cpu model,+name,-name,... option, function is CPUID level/function, i.e.
0xc0000001 according to your comment above, e?x is the output register
(eax/ebx/ecx/edx) which contains the bit which corresponds to the feature and
the value of this e?x attribute is the value of that bit. The right register
seems to be edx according to the qemu patch you mentioned above.

Finally, you may want to add new CPU models:

    <model name='...'>
      <vendor name='VIA'/>
      <feature name='...'/>
      ...
    </model>

this tells libvirt what features are supported by given CPU model. BTW, you
can inherit feature from older models using

    <model name='old'>
      <vendor name='VIA'/>
      ...
    </model>
    <model name='new'>
      <vendor name='VIA'/>
      <model name='old'/>
      ...
    </model>

this way model 'new' will support all features supported by model 'old' plus
features that are explicitly specified for model 'new'.

However, there are few things that may complicate the job.

First, I'm afraid we will need to come up with vendor features, that is
something like

    <feature name='...'>
      <vendor name='...'/>
      <cpuid .../>
    </feature>

which libvirt doesn't currently support but it smells like we'll need it so
that the new features are not misreported for non-VIA CPUs which might have
report something completely different there (I'm not sure it may actually
happen, though).

Another thing is that the CPU models should ideally be supported by qemu
(which doesn't seem to have any VIA models defined) but that's not strictly
required. If they are not supported by qemu, libvirt will use the new models
when reporting host CPU and convert them to another model that qemu supports
if a user asks for them in guest XML.

But the most important thing is that current qemu doesn't seem to even support
any of the new features in -cpu option. Only when qemu is started with
-cpu host (which libvirt doesn't support now but I'm working on patches for
it) the new VIA features are passed to kvm.

Comment 5 Cole Robinson 2016-03-23 14:57:52 UTC
The only thing to do here would be to extend cpu_map.xml, but it would just give nicer CPU model name reporting for capabilities output and cpu mode=host-model. The KVM and qemu bits that were added for VIA didn't add any public facing changes AFAICT (no new -cpu model or feature names), it was just about handling -cpu host better

I don't think the syntactic sugar for cpu_map.xml isn't that interesting to track here. If someone wants it I suggest sending a patch, otherwise it's just going to languish in this tracker forever. So closing as DEFERRED


Note You need to log in before you can comment on or make changes to this bug.