[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/2] Add new MSR modules (rdmsr/wrmsr)
|
From: |
Eric Snowberg |
|
Subject: |
Re: [PATCH v3 2/2] Add new MSR modules (rdmsr/wrmsr) |
|
Date: |
Thu, 21 Mar 2019 17:23:58 -0600 |
> On Mar 7, 2019, at 5:26 PM, Jesús Diéguez Fernández <address@hidden> wrote:
>
> In order to be able to read from and write to model-specific registers,
> two new modules are added. They are i386 specific, as the cpuid module.
>
> rdmsr module registers the command rdmsr that allows reading from a MSR.
> wrmsr module registers the command wrmsr that allows writing to a MSR.
>
> wrmsr module is disabled if UEFI secure boot is enabled.
>
> Please note that on SMP systems, interacting with a MSR that has a
> scope per hardware thread, implies that the value only applies to
> the particular cpu/core/thread that ran the command.
>
> Also, if you specify a reserved or unimplemented MSR address, it will
> cause a general protection exception (which is not currently being handled)
> and the system will reboot.
>
> Signed-off-by: Jesús Diéguez Fernández <address@hidden>
> ---
> docs/grub.texi | 53 +++++++++++++--
> grub-core/Makefile.core.def | 10 +++
> grub-core/commands/efi/shim_lock.c | 2 +-
> grub-core/commands/i386/rdmsr.c | 101 +++++++++++++++++++++++++++++
> grub-core/commands/i386/wrmsr.c | 92 ++++++++++++++++++++++++++
> include/grub/i386/rdmsr.h | 34 ++++++++++
> include/grub/i386/wrmsr.h | 32 +++++++++
> 7 files changed, 318 insertions(+), 6 deletions(-)
> create mode 100644 grub-core/commands/i386/rdmsr.c
> create mode 100644 grub-core/commands/i386/wrmsr.c
> create mode 100644 include/grub/i386/rdmsr.h
> create mode 100644 include/grub/i386/wrmsr.h
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index ecaba9d5c..4a636319f 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3931,6 +3931,7 @@ you forget a command, you can run the command
> @command{help}
> * play:: Play a tune
> * probe:: Retrieve device info
> * pxe_unload:: Unload the PXE environment
> +* rdmsr:: Read values from model-specific registers
> * read:: Read user input
> * reboot:: Reboot your computer
> * regexp:: Test if regular expression matches string
> @@ -3953,6 +3954,7 @@ you forget a command, you can run the command
> @command{help}
> * verify_detached:: Verify detached digital signature
> * videoinfo:: List available video modes
> @comment * xen_*:: Xen boot commands for AArch64
> +* wrmsr:: Write values to model-specific registers
> * xen_hypervisor:: Load xen hypervisor binary (only on AArch64)
> * xen_module:: Load xen modules for xen hypervisor (only on
> AArch64)
> @end menu
> @@ -4785,6 +4787,24 @@ This command is only available on PC BIOS systems.
> @end deffn
>
>
> address@hidden rdmsr
> address@hidden rdmsr
> +
> address@hidden Command: rdmsr 0xADDR [-v VARNAME]
> +Read a model-specific register at address 0xADDR. If the parameter
> address@hidden is used and an environment variable @var{VARNAME} is
> +given, set that environment variable to the value that was read.
> +
> +Please note that on SMP systems, reading from a MSR that has a
> +scope per hardware thread, implies that the value that is returned
> +only applies to the particular cpu/core/thread that runs the command.
> +
> +Also, if you specify a reserved or unimplemented MSR address, it will
> +cause a general protection exception (which is not currently being handled)
> +and the system will reboot.
> address@hidden deffn
> +
> +
> @node read
> @subsection read
>
> @@ -5223,6 +5243,21 @@ successfully. If validation fails, it is set to a
> non-zero value.
> List available video modes. If resolution is given, show only matching modes.
> @end deffn
>
> address@hidden wrmsr
> address@hidden wrmsr
> +
> address@hidden Command: wrmsr 0xADDR 0xVALUE
> +Write a 0xVALUE to a model-specific register at address 0xADDR.
> +
> +Please note that on SMP systems, writing to a MSR that has a scope
> +per hardware thread, implies that the value that is written
> +only applies to the particular cpu/core/thread that runs the command.
> +
> +Also, if you specify a reserved or unimplemented MSR address, it will
> +cause a general protection exception (which is not currently being handled)
> +and the system will reboot.
> address@hidden deffn
> +
> @node xen_hypervisor
> @subsection xen_hypervisor
>
> @@ -5716,11 +5751,11 @@ boot and the shim. This functionality is provided by
> the shim_lock module. It
> is recommend to build in this and other required modules into the
> @file{core.img}.
> All modules not stored in the @file{core.img} and the ACPI tables for the
> @command{acpi} command have to be signed, e.g. using PGP. Additionally, the
> address@hidden and the @command{memrw} commands are prohibited if the UEFI
> -secure boot is enabled. This is done due to security reasons. All above
> -mentioned requirements are enforced by the shim_lock module. And itself it
> -is a persistent module which means that it cannot be unloaded if it was
> -loaded into the memory.
> address@hidden, the @command{memrw} and the @command{wrmsr} commands are
> +prohibited if the UEFI secure boot is enabled. This is done due to
> +security reasons. All above mentioned requirements are enforced by the
> +shim_lock module. And itself it is a persistent module which means that
> +it cannot be unloaded if it was loaded into the memory.
>
> @node Measured Boot
> @section Measuring boot components
> @@ -5831,6 +5866,8 @@ to install to is specified, UUID is used instead as
> well.
> @item USB @tab yes @tab yes @tab yes @tab yes
> @item chainloader @tab local @tab yes @tab yes @tab no
> @item cpuid @tab partial @tab partial @tab partial @tab
> partial
> address@hidden rdmsr @tab partial @tab partial @tab partial
> @tab partial
> address@hidden wrmsr @tab partial @tab partial @tab partial
> @tab partial
> @item hints @tab guess @tab guess @tab guess @tab
> guess
> @item PCI @tab yes @tab yes @tab yes @tab yes
> @item badram @tab yes @tab yes @tab yes @tab yes
> @@ -5850,6 +5887,8 @@ to install to is specified, UUID is used instead as
> well.
> @item USB @tab yes @tab yes @tab yes
> @tab no
> @item chainloader @tab local @tab local @tab no
> @tab local
> @item cpuid @tab partial @tab partial @tab partial
> @tab no
> address@hidden rdmsr @tab partial @tab partial @tab
> partial @tab no
> address@hidden wrmsr @tab partial @tab partial @tab
> partial @tab no
> @item hints @tab guess @tab guess @tab good
> @tab guess
> @item PCI @tab yes @tab yes @tab yes
> @tab no
> @item badram @tab yes @tab yes @tab no
> @tab yes
> @@ -5869,6 +5908,8 @@ to install to is specified, UUID is used instead as
> well.
> @item USB @tab yes @tab no @tab no @tab no
> @item chainloader @tab yes @tab no @tab no @tab no
> @item cpuid @tab no @tab no @tab no @tab no
> address@hidden rdmsr @tab no @tab no @tab no
> @tab no
> address@hidden wrmsr @tab no @tab no @tab no
> @tab no
> @item hints @tab good @tab good @tab good @tab no
> @item PCI @tab yes @tab no @tab no @tab no
> @item badram @tab yes (*) @tab no @tab no @tab no
> @@ -5888,6 +5929,8 @@ to install to is specified, UUID is used instead as
> well.
> @item USB @tab N/A @tab yes @tab no
> @item chainloader @tab yes @tab no @tab yes
> @item cpuid @tab no @tab no @tab yes
> address@hidden rdmsr @tab no @tab no @tab yes
> address@hidden wrmsr @tab no @tab no @tab yes
> @item hints @tab guess @tab no @tab no
> @item PCI @tab no @tab no @tab no
> @item badram @tab yes (*) @tab no @tab no
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 58df4ab07..dbc7eb3a5 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2484,3 +2484,13 @@ module = {
> common = loader/i386/xen_file64.c;
> extra_dist = loader/i386/xen_fileXX.c;
> };
> +module = {
> + name = rdmsr;
> + common = commands/i386/rdmsr.c;
> + enable = x86;
> +};
> +module = {
> + name = wrmsr;
> + common = commands/i386/wrmsr.c;
> + enable = x86;
> +};
> diff --git a/grub-core/commands/efi/shim_lock.c
> b/grub-core/commands/efi/shim_lock.c
> index 83568cb2b..764098cfc 100644
> --- a/grub-core/commands/efi/shim_lock.c
> +++ b/grub-core/commands/efi/shim_lock.c
> @@ -43,7 +43,7 @@ static grub_efi_guid_t shim_lock_guid =
> GRUB_EFI_SHIM_LOCK_GUID;
> static grub_efi_shim_lock_protocol_t *sl;
>
> /* List of modules which cannot be loaded if UEFI secure boot mode is
> enabled. */
> -static const char * const disabled_mods[] = {"iorw", "memrw", NULL};
> +static const char * const disabled_mods[] = {"iorw", "memrw", "wrmsr", NULL};
>
> static grub_err_t
> shim_lock_init (grub_file_t io, enum grub_file_type type,
> diff --git a/grub-core/commands/i386/rdmsr.c b/grub-core/commands/i386/rdmsr.c
> new file mode 100644
> index 000000000..0ec91e2e5
> --- /dev/null
> +++ b/grub-core/commands/i386/rdmsr.c
> @@ -0,0 +1,101 @@
> +/* rdmsr.c - Read CPU model-specific registers */
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2019 Free Software Foundation, Inc.
> + * Based on gcc/gcc/config/i386/driver-i386.c
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/dl.h>
> +#include <grub/misc.h>
> +#include <grub/mm.h>
> +#include <grub/env.h>
> +#include <grub/command.h>
> +#include <grub/extcmd.h>
> +#include <grub/i386/rdmsr.h>
> +#include <grub/i18n.h>
> +#include <grub/cpu/cpuid.h>
> +
> +GRUB_MOD_LICENSE("GPLv3+");
> +
> +static grub_extcmd_t cmd_read;
> +
> +static const struct grub_arg_option options[] =
> +{
> + {0, 'v', 0, N_("Save read value into variable VARNAME."),
> + N_("VARNAME"), ARG_TYPE_STRING},
> + {0, 0, 0, 0, 0, 0}
> +};
> +
> +static grub_err_t
> +grub_cmd_msr_read (grub_extcmd_context_t ctxt, int argc, char **argv)
> +{
> + grub_uint32_t manufacturer[3], max_cpuid, a, b, c, features, addr;
> + grub_uint64_t value;
> + char *ptr;
> + char buf[sizeof("1122334455667788")];
> +
> + /* The CPUID instruction should be used to determine whether MSRs
> + are supported. (CPUID.01H:EDX[5] = 1) */
> + if (! grub_cpu_is_cpuid_supported ())
> + return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
> +
> + grub_cpuid (0, max_cpuid, manufacturer[0], manufacturer[2],
> manufacturer[1]);
> +
> + if (max_cpuid < 1)
> + return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
> +
> + grub_cpuid (1, a, b, c, features);
> +
> + if (! (features & (1 << 5)))
> + return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
> +
> + if (argc != 1)
> + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument
> expected"));
> +
> + grub_errno = GRUB_ERR_NONE;
> + ptr = argv[0];
> + addr = grub_strtoul (ptr, &ptr, 0);
> +
> + if (grub_errno != GRUB_ERR_NONE)
> + return grub_errno;
> + if (*ptr != '\0')
> + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument"));
> +
> + value = grub_msr_read (addr);
> +
> + if (ctxt->state[0].set)
> + {
> + grub_snprintf (buf, sizeof(buf), "%llx", value);
> + grub_env_set (ctxt->state[0].arg, buf);
> + }
> + else
> + grub_printf ("0x%llx\n", value);
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +GRUB_MOD_INIT(rdmsr)
> +{
> + cmd_read = grub_register_extcmd ("rdmsr", grub_cmd_msr_read, 0,
> + N_("ADDR"),
> + N_("Read a CPU model specific
> register."),
> + options);
> +}
> +
> +GRUB_MOD_FINI(rdmsr)
> +{
> + grub_unregister_extcmd (cmd_read);
> +}
> diff --git a/grub-core/commands/i386/wrmsr.c b/grub-core/commands/i386/wrmsr.c
> new file mode 100644
> index 000000000..8ebc21150
> --- /dev/null
> +++ b/grub-core/commands/i386/wrmsr.c
> @@ -0,0 +1,92 @@
> +/* wrmsr.c - Write CPU model-specific registers */
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2019 Free Software Foundation, Inc.
> + * Based on gcc/gcc/config/i386/driver-i386.c
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/dl.h>
> +#include <grub/misc.h>
> +#include <grub/mm.h>
> +#include <grub/env.h>
> +#include <grub/command.h>
> +#include <grub/extcmd.h>
> +#include <grub/i386/wrmsr.h>
> +#include <grub/i18n.h>
> +#include <grub/cpu/cpuid.h>
> +
> +GRUB_MOD_LICENSE("GPLv3+");
> +
> +static grub_command_t cmd_write;
> +
> +static grub_err_t
> +grub_cmd_msr_write (grub_command_t cmd, int argc, char **argv)
> +{
> + grub_uint32_t manufacturer[3], max_cpuid, a, b, c, features, addr;
> + grub_uint64_t value;
> + char *ptr;
> +
> + /* The CPUID instruction should be used to determine whether MSRs
> + are supported. (CPUID.01H:EDX[5] = 1) */
> + if (! grub_cpu_is_cpuid_supported ())
> + return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
> +
> + grub_cpuid (0, max_cpuid, manufacturer[0], manufacturer[2],
> manufacturer[1]);
> +
> + if (max_cpuid < 1)
> + return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
> +
> + grub_cpuid (1, a, b, c, features);
> +
> + if (! (features & (1 << 5)))
> + return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
> +
> + if (argc != 2)
> + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments
> expected"));
> +
> + grub_errno = GRUB_ERR_NONE;
> + ptr = argv[0];
> + addr = grub_strtoul (ptr, &ptr, 0);
> +
> + if (grub_errno != GRUB_ERR_NONE)
> + return grub_errno;
> + if (*ptr != '\0')
> + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument"));
> +
> + ptr = argv[1];
> + value = grub_strtoull (ptr, &ptr, 0);
> +
> + if (grub_errno != GRUB_ERR_NONE)
> + return grub_errno;
> + if (*ptr != '\0')
> + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument"));
> +
> + grub_msr_write (addr, value);
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +GRUB_MOD_INIT(wrmsr)
> +{
> + cmd_write = grub_register_command ("wrmsr", grub_cmd_msr_write,
> + N_("ADDR VALUE"),
> + N_("Write a value to a CPU model
> specific register."));
> +}
> +
> +GRUB_MOD_FINI(wrmsr)
> +{
> + grub_unregister_command (cmd_write);
> +}
> diff --git a/include/grub/i386/rdmsr.h b/include/grub/i386/rdmsr.h
> new file mode 100644
> index 000000000..f6d7b72ca
> --- /dev/null
> +++ b/include/grub/i386/rdmsr.h
> @@ -0,0 +1,34 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2019 Free Software Foundation, Inc.
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef GRUB_RDMSR_H
> +#define GRUB_RDMSR_H 1
> +
> +/* TODO: Add a general protection exception handler.
> + Accessing a reserved or unimplemented MSR address results in a GP#.
> */
> +
> +extern __inline grub_uint64_t grub_msr_read (grub_uint32_t msr_id)
I’m seeing a compile error with this patch:
In file included from commands/i386/rdmsr.c:29:0:
../include/grub/i386/rdmsr.h:27:29: error: no previous prototype for
_grub_msr_read_ [-Werror=missing-prototypes]
extern inline grub_uint64_t grub_msr_read (grub_uint32_t msr_id)
^
cc1: all warnings being treated as errors
> +{
> + grub_uint32_t low, high;
> +
> + asm volatile ( "rdmsr" : "=a"(low), "=d"(high) : "c"(msr_id) );
> +
> + return ((grub_uint64_t)high << 32) | low;
> +}
> +
> +#endif /* GRUB_RDMSR_H */
> diff --git a/include/grub/i386/wrmsr.h b/include/grub/i386/wrmsr.h
> new file mode 100644
> index 000000000..f20f5fdf2
> --- /dev/null
> +++ b/include/grub/i386/wrmsr.h
> @@ -0,0 +1,32 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2019 Free Software Foundation, Inc.
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef GRUB_WRMSR_H
> +#define GRUB_WRMSR_H 1
> +
> +/* TODO: Add a general protection exception handler.
> + Accessing a reserved or unimplemented MSR address results in a GP#.
> */
> +
> +extern __inline void grub_msr_write(grub_uint32_t msr_id, grub_uint64_t
> msr_value)
Also here:
In file included from commands/i386/wrmsr.c:29:0:
../include/grub/i386/wrmsr.h:27:20: error: no previous prototype for
_grub_msr_write_ [-Werror=missing-prototypes]
extern inline void grub_msr_write(grub_uint32_t msr_id, grub_uint64_t
msr_value)
^
cc1: all warnings being treated as errors
I believe it would be best to either provide a global definition for the
function, or declare the functions as static inline, or move them into the c
file, since neither is used elsewhere. If you would like me to fix it, let me
know the approach you would like to see done.
> +{
> + grub_uint32_t low = msr_value, high = msr_value >> 32;
> +
> + asm volatile ( "wrmsr" : : "c"(msr_id), "a"(low), "d"(high) );
> +}
> +
> +#endif /* GRUB_WRMSR_H */
> --
> 2.17.1
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel