[PATCH] smm: ignore bits 16,18-31 of SMM revision ID

[Paolo Bonzini]

Bits 16-31 of the SMM revision ID are feature bits.  We only need to
check that SMBASE relocation is supported, but do not care about other
features.  In particular, this allows the SMM I/O instruction restart
feature to be present.

Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com&gt;
---
 src/fw/smm.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/fw/smm.c b/src/fw/smm.c
index dabc677..6cb484e 100644
--- a/src/fw/smm.c
+++ b/src/fw/smm.c
@@ -18,8 +18,14 @@
 #include "util.h" // smm_setup
 #include "x86.h" // wbinvd
 
-#define SMM_REV_I32 0x00020000
-#define SMM_REV_I64 0x00020064
+/*
+ * Check SMM state save area format (bits 0-15) and require support
+ * for SMBASE relocation.
+ */
+#define SMM_REV_MASK 0x0002ffff
+
+#define SMM_REV_I32  0x00020000
+#define SMM_REV_I64  0x00020064
 
 struct smm_state {
     union {
@@ -62,9 +68,10 @@ handle_smi(u16 cs)
 
     if (smm == (void*)BUILD_SMM_INIT_ADDR) {
         // relocate SMBASE to 0xa0000
-        if (smm->cpu.i32.smm_rev == SMM_REV_I32) {
+        u32 rev = smm->cpu.i32.smm_rev & SMM_REV_MASK;
+        if (rev == SMM_REV_I32) {
             smm->cpu.i32.smm_base = BUILD_SMM_ADDR;
-        } else if (smm->cpu.i64.smm_rev == SMM_REV_I64) {
+        } else if (rev == SMM_REV_I64) {
             smm->cpu.i64.smm_base = BUILD_SMM_ADDR;
         } else {
             warn_internalerror();
-- 
2.3.5

[Kevin O'Connor]

On Wed, May 06, 2015 at 12:38:29PM +0200, Paolo Bonzini wrote:
 Bits 16-31 of the SMM revision ID are feature bits.  We only need to
 check that SMBASE relocation is supported, but do not care about other
 features.  In particular, this allows the SMM I/O instruction restart
 feature to be present.
Thanks - applied.

-Kevin

[Kevin O'Connor]

On Wed, May 06, 2015 at 12:38:29PM +0200, Paolo Bonzini wrote:
 Bits 16-31 of the SMM revision ID are feature bits.  We only need to
 check that SMBASE relocation is supported, but do not care about other
 features.  In particular, this allows the SMM I/O instruction restart
 feature to be present.
 
 Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com&gt;
 ---
  src/fw/smm.c | 15 +++++++++++----
  1 file changed, 11 insertions(+), 4 deletions(-)
 
 diff --git a/src/fw/smm.c b/src/fw/smm.c
 index dabc677..6cb484e 100644
 --- a/src/fw/smm.c
 +++ b/src/fw/smm.c
 @@ -18,8 +18,14 @@
  #include "util.h" // smm_setup
  #include "x86.h" // wbinvd
  
 -#define SMM_REV_I32 0x00020000
 -#define SMM_REV_I64 0x00020064
 +/*
 + * Check SMM state save area format (bits 0-15) and require support
 + * for SMBASE relocation.
 + */
 +#define SMM_REV_MASK 0x0002ffff
 +
 +#define SMM_REV_I32  0x00020000
 +#define SMM_REV_I64  0x00020064
  
  struct smm_state {
      union {
 @@ -62,9 +68,10 @@ handle_smi(u16 cs)
  
      if (smm == (void*)BUILD_SMM_INIT_ADDR) {
          // relocate SMBASE to 0xa0000
 -        if (smm->cpu.i32.smm_rev == SMM_REV_I32) {
 +        u32 rev = smm->cpu.i32.smm_rev & SMM_REV_MASK;
 +        if (rev == SMM_REV_I32) {
I noticed that this patch from May changes the revision check to use a
mask in the SMM init code, but doesn't change the check in the runtime
part of the handler.  Was this intentional?

-Kevin

[Paolo Bonzini]

On 30/07/2015 17:36, Kevin O'Connor wrote:
> >      if (smm == (void*)BUILD_SMM_INIT_ADDR) {
> >          // relocate SMBASE to 0xa0000
> > -        if (smm->cpu.i32.smm_rev == SMM_REV_I32) {
> > +        u32 rev = smm->cpu.i32.smm_rev & SMM_REV_MASK;
> > +        if (rev == SMM_REV_I32) {
 I noticed that this patch from May changes the revision check to use a
 mask in the SMM init code, but doesn't change the check in the runtime
 part of the handler.  Was this intentional?
No, it wasn't!  Good catch.

Paolo