On Thu, Jul 07, 2016 at 06:01:46PM +0200, Borislav Petkov wrote:
> On Thu, Jul 07, 2016 at 03:16:21PM +0200, Paolo Bonzini wrote:
> > Eduardo is the one to answer, but usually we add features to QEMU
> > before the processors are released (typically as soon as KVM supports
> > them). So with a new enough QEMU this in theory should not be
> > necessary.
> >
> > Adding a new feature that's not in a CPU model and that's not
> > associated to new state is really trivial:
>
> Cool.
>
> Btw, how about something like this?
>
> Specifically, I'd like to test RAS features on the new upcoming AMD
> Zen CPU and I've defined one from the stuff we know so far from kernel
> patches.
You mean KVM kernel patches? I assume the features require
additional KVM code to support them in guests. In that case, why
wouldn't the kernel return them in GET_SUPPORTED_CPUID? Then you
won't need filter=off.
>
> The "filter=off" thing I've added in case I want to disable
> x86_cpu_filter_features() but it works just fine without it when I boot
> with -cpu Zen. So I can remove it too.
>
> Would something like that be acceptable?
About filter=off: not sure. Do we really have valid use cases to
enable a feature even if the kernel reports it as unsupported in
GET_SUPPORTED_CPUID?
Specifically about the feature names, I have some question below:
>
> We can continue improving on this as features become known and even
> implement some functionality in qemu/kvm as time allows.
>
> ---
> From: Borislav Petkov <bp@xxxxxxx>
> Date: Tue, 5 Jul 2016 16:12:18 +0200
> Subject: [PATCH] Zen emu: first working version
>
> Boot with "-c Zen,filter=off" to disable CPUID bits filtering.
>
> Signed-off-by: Borislav Petkov <bp@xxxxxxx>
> ---
> target-i386/cpu.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> target-i386/cpu.h | 7 +++++++
> 2 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 3bd3cfc3ad16..cc9c97457387 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -307,6 +307,17 @@ static const char *cpuid_6_feature_name[] = {
> NULL, NULL, NULL, NULL,
> };
>
> +static const char *smca_feature_name[] = {
> + "overflow_recov", "succor", NULL, "smca",
Do those features introduce additional state that need migration
support? If they do, you need to add them to
feature_word_info[FEAT_8000_0007_EBX].unmigratable_flags until
migration support is implemented.
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> +};
> +
> #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
> #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
> CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
> @@ -449,6 +460,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> .cpuid_eax = 6, .cpuid_reg = R_EAX,
> .tcg_features = TCG_6_EAX_FEATURES,
> },
> + [FEAT_8000_0007_EBX] = {
> + .feat_names = smca_feature_name,
> + .cpuid_eax = 0x80000007,
> + .cpuid_reg = R_EBX,
> + },
> };
>
> typedef struct X86RegisterInfo32 {
[...]
--
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html