Re: [PATCH] target/i386: Remove MPX support on Snowridge CPU model

[Xiaoyao Li]

On Fri, 2019-10-11 at 09:57 -0300, Eduardo Habkost wrote:
> On Fri, Oct 11, 2019 at 03:18:44PM +0800, Xiaoyao Li wrote:
> > MPX support is being phased out by Intel. Following other CPU models
> > like Skylake, Icelake and Cascadelake, do not enable it by default.
> > 
> > Signed-off-by: Xiaoyao Li <address@hidden>
> > ---
> > I'm not sure is there anyone already use Snowridge CPU model and whether to
> > add it in pc_compat_4_1, since Snowridge has not been released yet.
> 
> We have a CPU model versioning mechanism now, please implement this using the
> new system.  e.g.:
> 
>         .versions = (X86CPUVersionDefinition[]) {
>             { .version = 1 },
>             {
>                 .version = 2,
>                 .props = (PropValue[]) {
>                     { "mpx", "off" },
>                     { /* end of list */ }
>                 }
>             },
>             { /* end of list */ }
>         }
> 
> Then machine-type compat properties won't be needed anymore.

Since Snowridge CPU hasn't been released yet and I guess there might be no real
user use this CPU model. So do we really need to add v2? Can we just remove the
mpx feature in the original version?

> > 
> > ---
> >  hw/i386/pc.c      | 4 +++-
> >  target/i386/cpu.c | 1 -
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index bcda50efcc23..97eb7ac32588 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -105,7 +105,9 @@ struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
> >  /* Physical Address of PVH entry point read from kernel ELF NOTE */
> >  static size_t pvh_start_addr;
> >  
> > -GlobalProperty pc_compat_4_1[] = {};
> > +GlobalProperty pc_compat_4_1[] = {
> > +    { "Snowridge" "-" TYPE_X86_CPU, "mpx", "on" },
> > +};
> >  const size_t pc_compat_4_1_len = G_N_ELEMENTS(pc_compat_4_1);
> >  
> >  GlobalProperty pc_compat_4_0[] = {};
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 44f1bbdcac76..885bea76205d 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -2762,7 +2762,6 @@ static X86CPUDefinition builtin_x86_defs[] = {
> >              CPUID_7_0_EBX_FSGSBASE |
> >              CPUID_7_0_EBX_SMEP |
> >              CPUID_7_0_EBX_ERMS |
> > -            CPUID_7_0_EBX_MPX |  /* missing bits 13, 15 */
> >              CPUID_7_0_EBX_RDSEED |
> >              CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_CLFLUSHOPT |
> >              CPUID_7_0_EBX_CLWB |
> > -- 
> > 2.19.1
> > 
> 
>