Re: [PATCH v2 06/13] KVM: x86: save/load state on SMM switch

[Paolo Bonzini]

On 03/06/2015 21:02, Radim KrÄmÃÅ wrote:
>> +	r = kvm_write_guest(vcpu->kvm, vcpu->arch.smbase + 0xfe00, buf, sizeof(buf));
> 
> The state is saved in SMRAM, but we are accessing it using the non-SMM
> address space ... how did it pass testing?
> (Restore is using SMM address space, so I'm guessing that the mapping
>  from QEMU wasn't really utilizing two separate address spaces.)

At this point of the series there are no separate address spaces yet.
Patch 10 then changes it everywhere:

@@ -6558,7 +6558,7 @@ static void process_smi(struct kvm_vcpu *vcpu)
 	else
 		process_smi_save_state_32(vcpu, buf);
 
-	r = kvm_write_guest(vcpu->kvm, vcpu->arch.smbase + 0xfe00, buf, sizeof(buf));
+	r = kvm_vcpu_write_guest(vcpu, vcpu->arch.smbase + 0xfe00, buf, sizeof(buf));
 	if (r < 0)
 		return;

Why did I order it this way?  Because it is already possible to test
this code with the default SMBASE of 0x30000, and it is already
possible to run the full firmware if you hack it not to close SMRAM
(for this I used q35's high SMRAM).  It is not possible to test the
code partially if you first add the two address spaces, and only
implement the world switch second.

Thanks,

Paolo


>> +	if (r < 0)
>> +		return;
> 
> And if we fail to write it, is there other option than throwing an error
> to userspace?  (Unset HF_SMM_MASK and pretend that nothing happened
> doesn't find much support in docs.)
> 
> Thanks.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/