Re: [SeaBIOS] [PATCH] fw/msr_feature_control: add support to set MSR_IA32_FEATURE_CONTROL

[Paolo Bonzini]

On 16/06/2016 13:49, Haozhong Zhang wrote:
> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> index 8ed4380..640ee4c 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -153,6 +153,9 @@ qemu_platform_setup(void)
>      mtrr_setup();
>      smp_setup();
> // Initialize MSR_IA32_FEATURE_CONTROL
> msr_feature_control_setup();
> // Create bios tables
>    pirtable_setup();
>    mptable_setup();

This must run before smp_setup(), because it is smp_setup() that calls
handle_smp() on the APs.
> diff --git a/src/fw/msr_feature_control.c b/src/fw/msr_feature_control.c
> new file mode 100644
> index 0000000..35d4ab8
> --- /dev/null
> +++ b/src/fw/msr_feature_control.c
> @@ -0,0 +1,16 @@
> +#include "util.h" // msr_feature_control_setup
> +#include "x86.h" // wrmsr
> +#include "romfile.h" // romfile_find
>
> +u64 feature_control_bits;
>
> +void msr_feature_control_setup(void)
> +{
> struct romfile_s *f = romfile_find("etc/msr_feature_control");
> if (!f)
>    return;
> f->copy(f, &feature_control_bits, sizeof(feature_control_bits));
> if (feature_control_bits)
>    wrmsr(MSR_IA32_FEATURE_CONTROL, feature_control_bits);

You can use wrmsr_smp and avoid the change below to handle_smp().  It
also removes the need for the feature_control_bits global variable.

Thanks,

Paolo
> +}
> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> index 8ed4380..640ee4c 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -153,6 +153,9 @@ qemu_platform_setup(void)
>      mtrr_setup();
>      smp_setup();
> // Initialize MSR_IA32_FEATURE_CONTROL
> msr_feature_control_setup();
> // Create bios tables
>    pirtable_setup();
>    mptable_setup();
>
> diff --git a/src/fw/smp.c b/src/fw/smp.c
> index 579acdb..8b57bf4 100644
> --- a/src/fw/smp.c
> +++ b/src/fw/smp.c
> @@ -10,7 +10,7 @@
>  #include "output.h" // dprintf
>  #include "romfile.h" // romfile_loadint
>  #include "stacks.h" // yield
> -#include "util.h" // smp_setup
> +#include "util.h" // smp_setup, msr_feature_control_setup
>  #include "x86.h" // wrmsr
>
> #define APIC_ICR_LOW ((u8*)BUILD_APIC_ADDR + 0x300)
> @@ -63,6 +63,10 @@ handle_smp(void)
>      for (i=0; i<smp_mtrr_count; i++)
>          wrmsr(smp_mtrr[i].index, smp_mtrr[i].val);
> // MSR_IA32_FEATURE_CONTROL setup
> if (feature_control_bits)
>    wrmsr(MSR_IA32_FEATURE_CONTROL, feature_control_bits);
> // Set bit on FoundAPICIDs
>    FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
>
> diff --git a/src/util.h b/src/util.h
> index 7b41207..fc8e59d 100644
> --- a/src/util.h
> +++ b/src/util.h
> @@ -103,6 +103,11 @@ int csm_bootprio_pci(struct pci_device *pci);
>  // fw/mptable.c
>  void mptable_setup(void);
>
> +// fw/msr_feature_control.c
> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> +extern u64 feature_control_bits;
> +void msr_feature_control_setup(void);
>
> // fw/mtrr.c
>  void mtrr_setup(void);