[Qemu-devel] [RFC PATCH] i386: Remove ospke CPUID flag name

[Qemu-devel] [RFC PATCH] i386: Remove ospke CPUID flag name

[Eduardo Habkost]

RFC NOTE: Paolo, Richard, as far as I can see, there's no point
in enabling OSPKE in user-mode QEMU.  Do you confirm that?

OSPKE is not a static feature flag: it changes dynamically at
runtime depending on CR4, and it was never configurable: KVM
never returned OSPKE on GET_SUPPORTED_CPUID, and on TCG enables
it automatically if CR4_PKE_MASK is set.

Remove OSPKE from the feature name array so users don't try to
configure it manually.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/cpu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 94260412e2..f101771ac0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -564,7 +564,8 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
           CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
           CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
           CPUID_7_0_EBX_RDSEED */
-#define TCG_7_0_ECX_FEATURES (CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE | \
+#define TCG_7_0_ECX_FEATURES (CPUID_7_0_ECX_PKU | \
+          /* CPUID_7_0_ECX_OSPKE is dynamic */ \
           CPUID_7_0_ECX_LA57)
 #define TCG_7_0_EDX_FEATURES 0
 #define TCG_APM_FEATURES 0
@@ -781,7 +782,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
     [FEAT_7_0_ECX] = {
         .feat_names = {
             NULL, "avx512vbmi", "umip", "pku",
-            "ospke", NULL, "avx512vbmi2", NULL,
+            NULL /* ospke */, NULL, "avx512vbmi2", NULL,
             "gfni", "vaes", "vpclmulqdq", "avx512vnni",
             "avx512bitalg", NULL, "avx512-vpopcntdq", NULL,
             "la57", NULL, NULL, NULL,
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [RFC PATCH] i386: Remove ospke CPUID flag name

[Richard Henderson]

On 06/11/2018 10:37 AM, Eduardo Habkost wrote:
> RFC NOTE: Paolo, Richard, as far as I can see, there's no point
> in enabling OSPKE in user-mode QEMU.  Do you confirm that?
> 
> OSPKE is not a static feature flag: it changes dynamically at
> runtime depending on CR4, and it was never configurable: KVM
> never returned OSPKE on GET_SUPPORTED_CPUID, and on TCG enables
> it automatically if CR4_PKE_MASK is set.
> 
> Remove OSPKE from the feature name array so users don't try to
> configure it manually.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target/i386/cpu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [Qemu-devel] [RFC PATCH] i386: Remove ospke CPUID flag name

[Paolo Bonzini]

On 11/06/2018 22:37, Eduardo Habkost wrote:
> RFC NOTE: Paolo, Richard, as far as I can see, there's no point
> in enabling OSPKE in user-mode QEMU.  Do you confirm that?
> 
> OSPKE is not a static feature flag: it changes dynamically at
> runtime depending on CR4, and it was never configurable: KVM
> never returned OSPKE on GET_SUPPORTED_CPUID, and on TCG enables
> it automatically if CR4_PKE_MASK is set.
> 
> Remove OSPKE from the feature name array so users don't try to
> configure it manually.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Yes, it's the same as OSXSAVE.  Thanks!

Paolo

Re: [Qemu-devel] [RFC PATCH] i386: Remove ospke CPUID flag name

[Eduardo Habkost]

On Tue, Jun 12, 2018 at 08:55:55AM +0200, Paolo Bonzini wrote:
> On 11/06/2018 22:37, Eduardo Habkost wrote:
> > RFC NOTE: Paolo, Richard, as far as I can see, there's no point
> > in enabling OSPKE in user-mode QEMU.  Do you confirm that?
> > 
> > OSPKE is not a static feature flag: it changes dynamically at
> > runtime depending on CR4, and it was never configurable: KVM
> > never returned OSPKE on GET_SUPPORTED_CPUID, and on TCG enables
> > it automatically if CR4_PKE_MASK is set.
> > 
> > Remove OSPKE from the feature name array so users don't try to
> > configure it manually.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Yes, it's the same as OSXSAVE.  Thanks!

CR4_OSXSAVE_MASK is automatically enabled on user-mode QEMU,
though.

My question is if it would make any sense to enable CR4_PKE_MASK
too.

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH] i386: Remove ospke CPUID flag name

[Paolo Bonzini]

On 12/06/2018 17:01, Eduardo Habkost wrote:
>>>
>>> Remove OSPKE from the feature name array so users don't try to
>>> configure it manually.
>>>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> Yes, it's the same as OSXSAVE.  Thanks!
> CR4_OSXSAVE_MASK is automatically enabled on user-mode QEMU,
> though.
> 
> My question is if it would make any sense to enable CR4_PKE_MASK
> too.

If you mean OSPKE, then yes---if PKU is available.  Likewise, OSXSAVE
should only be enabled if XSAVE is available.

Paolo

Re: [Qemu-devel] [RFC PATCH] i386: Remove ospke CPUID flag name

[Eduardo Habkost]

On Tue, Jun 12, 2018 at 05:12:58PM +0200, Paolo Bonzini wrote:
> On 12/06/2018 17:01, Eduardo Habkost wrote:
> >>>
> >>> Remove OSPKE from the feature name array so users don't try to
> >>> configure it manually.
> >>>
> >>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >> Yes, it's the same as OSXSAVE.  Thanks!
> > CR4_OSXSAVE_MASK is automatically enabled on user-mode QEMU,
> > though.
> > 
> > My question is if it would make any sense to enable CR4_PKE_MASK
> > too.
> 
> If you mean OSPKE, then yes---if PKU is available.  Likewise, OSXSAVE
> should only be enabled if XSAVE is available.

Yeah, I mean enabling it only if PKU is available, like we
already do with OSXAVE/XSAVE.

But we don't do it today, so enabling it automatically in
CONFIG_USER_ONLY would be a new feature.  Would it be useful for
anything, though?

I'm asking that to find out if somebody could be already using
"-cpu ...,+ospke" with user-mode QEMU today (which this patch
would break).  If RDPKRU/WRPKRU is useless under user-mode QEMU,
than we don't need to worry about that.

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH] i386: Remove ospke CPUID flag name

[Paolo Bonzini]

On 12/06/2018 20:25, Eduardo Habkost wrote:
> On Tue, Jun 12, 2018 at 05:12:58PM +0200, Paolo Bonzini wrote:
>> On 12/06/2018 17:01, Eduardo Habkost wrote:
>>>>>
>>>>> Remove OSPKE from the feature name array so users don't try to
>>>>> configure it manually.
>>>>>
>>>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>>> Yes, it's the same as OSXSAVE.  Thanks!
>>> CR4_OSXSAVE_MASK is automatically enabled on user-mode QEMU,
>>> though.
>>>
>>> My question is if it would make any sense to enable CR4_PKE_MASK
>>> too.
>>
>> If you mean OSPKE, then yes---if PKU is available.  Likewise, OSXSAVE
>> should only be enabled if XSAVE is available.
> 
> Yeah, I mean enabling it only if PKU is available, like we
> already do with OSXAVE/XSAVE.
> 
> But we don't do it today, so enabling it automatically in
> CONFIG_USER_ONLY would be a new feature.  Would it be useful for
> anything, though?
> 
> I'm asking that to find out if somebody could be already using
> "-cpu ...,+ospke" with user-mode QEMU today (which this patch
> would break).  If RDPKRU/WRPKRU is useless under user-mode QEMU,
> than we don't need to worry about that.

Hmm, actually there are two more things to consider for user-mode emulation.

First, QEMU doesn't support any of pkey_mprotect, pkey_alloc, pkey_free,
so it should probably never set OSPKE.

Second, for user-mode emulation it makes sense to allow flipping of
OSPKE and OSXSAVE, because that corresponds to different behaviors of
the underlying kernels.  There have been bugs in fact with programs that
incorrectly tested XSAVE instead of OSXSAVE, so it's worthwhile to let
the user test both configurations.

So to sum up, the default for QEMU user-mode emulation should be
OSXSAVE=XSAVE and OSPKE=0.

Paolo

Re: [Qemu-devel] [RFC PATCH] i386: Remove ospke CPUID flag name

[Eduardo Habkost]

On Wed, Jun 13, 2018 at 07:01:23PM +0200, Paolo Bonzini wrote:
> On 12/06/2018 20:25, Eduardo Habkost wrote:
> > On Tue, Jun 12, 2018 at 05:12:58PM +0200, Paolo Bonzini wrote:
> >> On 12/06/2018 17:01, Eduardo Habkost wrote:
> >>>>>
> >>>>> Remove OSPKE from the feature name array so users don't try to
> >>>>> configure it manually.
> >>>>>
> >>>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >>>> Yes, it's the same as OSXSAVE.  Thanks!
> >>> CR4_OSXSAVE_MASK is automatically enabled on user-mode QEMU,
> >>> though.
> >>>
> >>> My question is if it would make any sense to enable CR4_PKE_MASK
> >>> too.
> >>
> >> If you mean OSPKE, then yes---if PKU is available.  Likewise, OSXSAVE
> >> should only be enabled if XSAVE is available.
> > 
> > Yeah, I mean enabling it only if PKU is available, like we
> > already do with OSXAVE/XSAVE.
> > 
> > But we don't do it today, so enabling it automatically in
> > CONFIG_USER_ONLY would be a new feature.  Would it be useful for
> > anything, though?
> > 
> > I'm asking that to find out if somebody could be already using
> > "-cpu ...,+ospke" with user-mode QEMU today (which this patch
> > would break).  If RDPKRU/WRPKRU is useless under user-mode QEMU,
> > than we don't need to worry about that.
> 
> Hmm, actually there are two more things to consider for user-mode emulation.
> 
> First, QEMU doesn't support any of pkey_mprotect, pkey_alloc, pkey_free,
> so it should probably never set OSPKE.

This means "-cpu ...,+ospke" is useless today and we can safely
remove support for it, right?

> Second, for user-mode emulation it makes sense to allow flipping of
> OSPKE and OSXSAVE, because that corresponds to different behaviors of
> the underlying kernels.  There have been bugs in fact with programs that
> incorrectly tested XSAVE instead of OSXSAVE, so it's worthwhile to let
> the user test both configurations.

However, "-cpu ...,-osxsave" has no effect today (user-mode QEMU
unconditionally sets OSXSAVE), so that would be a new feature.

I assume this means I don't need to drop the osxsave patch from
my queue, either.

> So to sum up, the default for QEMU user-mode emulation should be
> OSXSAVE=XSAVE and OSPKE=0.

Thanks!

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH] i386: Remove ospke CPUID flag name

[Paolo Bonzini]

On 13/06/2018 19:16, Eduardo Habkost wrote:
>> Second, for user-mode emulation it makes sense to allow flipping of
>> OSPKE and OSXSAVE, because that corresponds to different behaviors of
>> the underlying kernels.  There have been bugs in fact with programs that
>> incorrectly tested XSAVE instead of OSXSAVE, so it's worthwhile to let
>> the user test both configurations.
>
> However, "-cpu ...,-osxsave" has no effect today (user-mode QEMU
> unconditionally sets OSXSAVE), so that would be a new feature.

Yes, understood.

Paolo

> I assume this means I don't need to drop the osxsave patch from
> my queue, either.
> 
>> So to sum up, the default for QEMU user-mode emulation should be
>> OSXSAVE=XSAVE and OSPKE=0.
> Thanks!