From: Joao Martins <joao.m.martins@oracle.com>
To: John Allen <john.allen@amd.com>, Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, yazen.ghannam@amd.com,
michael.roth@amd.com, babu.moger@amd.com,
william.roche@oracle.com
Subject: Re: [PATCH 1/2] i386: Add support for SUCCOR feature
Date: Thu, 20 Jul 2023 14:29:20 +0100 [thread overview]
Message-ID: <9380b925-26c9-99a6-2cc8-9a64c143dde7@oracle.com> (raw)
In-Reply-To: <ZK761jK1wB62T3xv@johallen-workstation>
On 12/07/2023 20:11, John Allen wrote:
> On Fri, Jul 07, 2023 at 04:25:22PM +0200, Paolo Bonzini wrote:
>> On 7/6/23 21:40, John Allen wrote:
>>> case 0x80000007:
>>> *eax = 0;
>>> - *ebx = 0;
>>> + *ebx = env->features[FEAT_8000_0007_EBX] | CPUID_8000_0007_EBX_SUCCOR;
>>> *ecx = 0;
>>> *edx = env->features[FEAT_8000_0007_EDX];
>>> break;
>>
>> I agree that it needs no hypervisor support, but Babu is right that you
>> cannot add it unconditionally (especially not on Intel processors).
>>
>> You can special case CPUID_8000_0007_EBX_SUCCOR in
>> kvm_arch_get_supported_cpuid() so that it is added even on old kernels.
>> There are already several such cases. Adding it to KVM is nice to have
>> anyway, so please send a patch for that.
>
> By adding it to KVM do you mean adding a patch to the kernel to expose
> the cpuid bit? Or do you mean just adding the special case to
> kvm_arch_get_supported_cpuid?
>
> For the kvm_arch_get_supported_cpuid case, I don't understand how this
> would be different from unconditionally exposing the bit as done above.
> Can you help me understand what you have in mind for this?
>
> I might add a case like below:
> ...
> } else if (function == 0x80000007 && reg == R_EBX) {
> ret |= CPUID_8000_0007_EBX_SUCCOR;
> ...
>
IIUC the thinking here is to cover a hypervisor that doesn't advertise SUCCOR
via supported cpuid from the kernel, by adding that elif statement above.
So when a CPU model is configured, the filtering of cpuid leafs logic
takes place, it doesn't "dissappear" when the values from the kernel isn't set.
Otherwise, if it's advertised by the kernel, everything goes as normal in this
filtering logic and everything works.
This presumes that you set "succor" feature string in the CPUID model definition
instead of unilaterally defining it like this patch. Such that only AMD CPU
models advertise the feature instead of everybody.
> If we wanted to only expose the bit for AMD cpus, we would then need to
> call IS_AMD_CPU with the CPUX86State as a paramter which would mean that
> kvm_arch_get_supported_cpuid and all of its callers would need to take
> the CPUX86State as a parameter. Is there another way to differentiate
> between AMD and Intel cpus in this case?
>
There might be an implicit convetion here where the caller is the one selecting
all the bits -- so kvm_arch_get_supported_cpuid won't care to validate which
vendor supports a given bit. Usually steered down via CPU model names (-cpu
Genoa), or named CPU features (-cpu Genoa,-feature-name) e.g. Nothing stops you
from trying to use a Intel CPUID on a AMD host. So the IS_AMD_CPU inside
kvm_arch_get_supported_cpuid doesn't seem to matter
Joao
next prev parent reply other threads:[~2023-07-20 13:30 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-06 19:40 [PATCH 0/2] Fix MCE handling on AMD hosts John Allen
2023-07-06 19:40 ` [PATCH 1/2] i386: Add support for SUCCOR feature John Allen
2023-07-06 20:22 ` Moger, Babu
2023-07-06 21:07 ` Joao Martins
2023-07-07 14:25 ` Paolo Bonzini
2023-07-12 19:11 ` John Allen
2023-07-20 13:29 ` Joao Martins [this message]
2023-07-06 19:40 ` [PATCH 2/2] i386: Fix MCE support for AMD hosts John Allen
2023-07-06 21:07 ` Joao Martins
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9380b925-26c9-99a6-2cc8-9a64c143dde7@oracle.com \
--to=joao.m.martins@oracle.com \
--cc=babu.moger@amd.com \
--cc=john.allen@amd.com \
--cc=michael.roth@amd.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=william.roche@oracle.com \
--cc=yazen.ghannam@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).