Re: [PATCH 16/22] x86: mtrr: Use MP calls to list the MTRRs

2020-06-10 Thread Wolfgang Wallner
Hi Simon,

-"Simon Glass"  schrieb: -
> Betreff: [PATCH 16/22] x86: mtrr: Use MP calls to list the MTRRs
> 
> Update the mtrr command to use mp_run_on_cpus() to obtain its information.
> Since the selected CPU is the boot CPU this does not change the result,
> but it sets the stage for supporting other CPUs.
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  arch/x86/cpu/mtrr.c | 11 +++
>  arch/x86/include/asm/mtrr.h | 30 ++
>  cmd/x86/mtrr.c  | 25 +
>  3 files changed, 62 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c
> index 7ec077d..11f3ef08172 100644
> --- a/arch/x86/cpu/mtrr.c
> +++ b/arch/x86/cpu/mtrr.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -63,6 +64,16 @@ static void set_var_mtrr(uint reg, uint type, uint64_t 
> start, uint64_t size)
>   wrmsrl(MTRR_PHYS_MASK_MSR(reg), mask | MTRR_PHYS_MASK_VALID);
>  }
>  
> +void mtrr_save_all(struct mtrr_info *info)

Nit: This is just a personal view, but to me, "save" sounds like writing to
an MTRR. But this function reads them. How about calling it "mtrr_read_all"?

> +{
> + int i;
> +
> + for (i = 0; i < MTRR_COUNT; i++) {
> + info->mtrr[i].base = native_read_msr(MTRR_PHYS_BASE_MSR(i));
> + info->mtrr[i].mask = native_read_msr(MTRR_PHYS_MASK_MSR(i));
> + }
> +}
> +
>  int mtrr_commit(bool do_caches)
>  {
>   struct mtrr_request *req = gd->arch.mtrr_req;
> diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h
> index 212a699c1b2..476d6f8a9cf 100644
> --- a/arch/x86/include/asm/mtrr.h
> +++ b/arch/x86/include/asm/mtrr.h
> @@ -70,6 +70,26 @@ struct mtrr_state {
>   bool enable_cache;
>  };
>  
> +/**
> + * struct mtrr - Information about a single MTRR
> + *
> + * @base: Base address and MTRR_BASE_TYPE_MASK
> + * @mask: Mask and MTRR_PHYS_MASK_VALID
> + */
> +struct mtrr {
> + u64 base;
> + u64 mask;
> +};
> +
> +/**
> + * struct mtrr_info - Information about all MTRRs
> + *
> + * @mtrr: Information about each mtrr
> + */
> +struct mtrr_info {
> + struct mtrr mtrr[MTRR_COUNT];
> +};
> +
>  /**
>   * mtrr_open() - Prepare to adjust MTRRs
>   *
> @@ -129,6 +149,16 @@ int mtrr_commit(bool do_caches);
>   */
>  int mtrr_set_next_var(uint type, uint64_t base, uint64_t size);
>  
> +/**
> + * mtrr_save_all() - Save all the MTRRs
> + *
> + * This writes all MTRRs from the boot CPU into a struct so they can be 
> loaded
> + * onto other CPUs
> + *
> + * @info: Place to put the MTRR info
> + */
> +void mtrr_save_all(struct mtrr_info *info);
> +
>  #endif
>  
>  #if ((CONFIG_XIP_ROM_SIZE & (CONFIG_XIP_ROM_SIZE - 1)) != 0)
> diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
> index 5d25c5802af..01197044452 100644
> --- a/cmd/x86/mtrr.c
> +++ b/cmd/x86/mtrr.c
> @@ -5,7 +5,9 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
>  
>  static const char *const mtrr_type_name[MTRR_TYPE_COUNT] = {
> @@ -18,19 +20,32 @@ static const char *const mtrr_type_name[MTRR_TYPE_COUNT] 
> = {
>   "Back",
>  };
>  
> -static int do_mtrr_list(void)
> +static void save_mtrrs(void *arg)
>  {
> + struct mtrr_info *info = arg;
> +
> + mtrr_save_all(info);
> +}
> +
> +static int do_mtrr_list(int cpu_select)
> +{
> + struct mtrr_info info;
> + int ret;
>   int i;
>  
>   printf("Reg Valid Write-type   %-16s %-16s %-16s\n", "Base   ||",
>  "Mask   ||", "Size   ||");
> + memset(, '\0', sizeof(info));
> + ret = mp_run_on_cpus(cpu_select, save_mtrrs, );
> + if (ret)
> + return log_msg_ret("run", ret);
>   for (i = 0; i < MTRR_COUNT; i++) {
>   const char *type = "Invalid";
>   uint64_t base, mask, size;
>   bool valid;
>  
> - base = native_read_msr(MTRR_PHYS_BASE_MSR(i));
> - mask = native_read_msr(MTRR_PHYS_MASK_MSR(i));
> + base = info.mtrr[i].base;
> + mask = info.mtrr[i].mask;
>   size = ~mask & ((1ULL << CONFIG_CPU_ADDR_BITS) - 1);
>   size |= (1 << 12) - 1;
>   size += 1;
> @@ -102,11 +117,13 @@ static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int 
> argc,
>  char *const argv[])
>  {
>   const char *cmd;
> + int cpu_select;
>   uint reg;
>  
> + cpu_select = MP_SELECT_BSP;
>   cmd = argv[1];
>   if (argc < 2 || *cmd == 'l')
> - return do_mtrr_list();
> + return do_mtrr_list(cpu_select);
>   argc -= 2;
>   argv += 2;
>   if (argc <= 0)
> -- 
> 2.27.0.rc0.183.gde8f92d652-goog

Reviewed-by: Wolfgang Wallner 


[PATCH 16/22] x86: mtrr: Use MP calls to list the MTRRs

2020-05-21 Thread Simon Glass
Update the mtrr command to use mp_run_on_cpus() to obtain its information.
Since the selected CPU is the boot CPU this does not change the result,
but it sets the stage for supporting other CPUs.

Signed-off-by: Simon Glass 
---

 arch/x86/cpu/mtrr.c | 11 +++
 arch/x86/include/asm/mtrr.h | 30 ++
 cmd/x86/mtrr.c  | 25 +
 3 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c
index 7ec077d..11f3ef08172 100644
--- a/arch/x86/cpu/mtrr.c
+++ b/arch/x86/cpu/mtrr.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -63,6 +64,16 @@ static void set_var_mtrr(uint reg, uint type, uint64_t 
start, uint64_t size)
wrmsrl(MTRR_PHYS_MASK_MSR(reg), mask | MTRR_PHYS_MASK_VALID);
 }
 
+void mtrr_save_all(struct mtrr_info *info)
+{
+   int i;
+
+   for (i = 0; i < MTRR_COUNT; i++) {
+   info->mtrr[i].base = native_read_msr(MTRR_PHYS_BASE_MSR(i));
+   info->mtrr[i].mask = native_read_msr(MTRR_PHYS_MASK_MSR(i));
+   }
+}
+
 int mtrr_commit(bool do_caches)
 {
struct mtrr_request *req = gd->arch.mtrr_req;
diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h
index 212a699c1b2..476d6f8a9cf 100644
--- a/arch/x86/include/asm/mtrr.h
+++ b/arch/x86/include/asm/mtrr.h
@@ -70,6 +70,26 @@ struct mtrr_state {
bool enable_cache;
 };
 
+/**
+ * struct mtrr - Information about a single MTRR
+ *
+ * @base: Base address and MTRR_BASE_TYPE_MASK
+ * @mask: Mask and MTRR_PHYS_MASK_VALID
+ */
+struct mtrr {
+   u64 base;
+   u64 mask;
+};
+
+/**
+ * struct mtrr_info - Information about all MTRRs
+ *
+ * @mtrr: Information about each mtrr
+ */
+struct mtrr_info {
+   struct mtrr mtrr[MTRR_COUNT];
+};
+
 /**
  * mtrr_open() - Prepare to adjust MTRRs
  *
@@ -129,6 +149,16 @@ int mtrr_commit(bool do_caches);
  */
 int mtrr_set_next_var(uint type, uint64_t base, uint64_t size);
 
+/**
+ * mtrr_save_all() - Save all the MTRRs
+ *
+ * This writes all MTRRs from the boot CPU into a struct so they can be loaded
+ * onto other CPUs
+ *
+ * @info: Place to put the MTRR info
+ */
+void mtrr_save_all(struct mtrr_info *info);
+
 #endif
 
 #if ((CONFIG_XIP_ROM_SIZE & (CONFIG_XIP_ROM_SIZE - 1)) != 0)
diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
index 5d25c5802af..01197044452 100644
--- a/cmd/x86/mtrr.c
+++ b/cmd/x86/mtrr.c
@@ -5,7 +5,9 @@
 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 
 static const char *const mtrr_type_name[MTRR_TYPE_COUNT] = {
@@ -18,19 +20,32 @@ static const char *const mtrr_type_name[MTRR_TYPE_COUNT] = {
"Back",
 };
 
-static int do_mtrr_list(void)
+static void save_mtrrs(void *arg)
 {
+   struct mtrr_info *info = arg;
+
+   mtrr_save_all(info);
+}
+
+static int do_mtrr_list(int cpu_select)
+{
+   struct mtrr_info info;
+   int ret;
int i;
 
printf("Reg Valid Write-type   %-16s %-16s %-16s\n", "Base   ||",
   "Mask   ||", "Size   ||");
+   memset(, '\0', sizeof(info));
+   ret = mp_run_on_cpus(cpu_select, save_mtrrs, );
+   if (ret)
+   return log_msg_ret("run", ret);
for (i = 0; i < MTRR_COUNT; i++) {
const char *type = "Invalid";
uint64_t base, mask, size;
bool valid;
 
-   base = native_read_msr(MTRR_PHYS_BASE_MSR(i));
-   mask = native_read_msr(MTRR_PHYS_MASK_MSR(i));
+   base = info.mtrr[i].base;
+   mask = info.mtrr[i].mask;
size = ~mask & ((1ULL << CONFIG_CPU_ADDR_BITS) - 1);
size |= (1 << 12) - 1;
size += 1;
@@ -102,11 +117,13 @@ static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int 
argc,
   char *const argv[])
 {
const char *cmd;
+   int cpu_select;
uint reg;
 
+   cpu_select = MP_SELECT_BSP;
cmd = argv[1];
if (argc < 2 || *cmd == 'l')
-   return do_mtrr_list();
+   return do_mtrr_list(cpu_select);
argc -= 2;
argv += 2;
if (argc <= 0)
-- 
2.27.0.rc0.183.gde8f92d652-goog