On 05.11.19 15:11, Janosch Frank wrote:
On 11/5/19 2:55 PM, David Hildenbrand wrote:
On 05.11.19 13:39, Janosch Frank wrote:
On 11/5/19 1:01 PM, Christian Borntraeger wrote:
On 04.11.19 12:25, David Hildenbrand wrote:
On 24.10.19 13:40, Janosch Frank wrote:
Guest registers for protected guests are stored at offset 0x380.
Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
---
arch/s390/include/asm/kvm_host.h | 4 +++-
arch/s390/kvm/kvm-s390.c | 11 +++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 0ab309b7bf4c..5deabf9734d9 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -336,7 +336,9 @@ struct kvm_s390_itdb {
struct sie_page {
struct kvm_s390_sie_block sie_block;
struct mcck_volatile_info mcck_info; /* 0x0200 */
- __u8 reserved218[1000]; /* 0x0218 */
+ __u8 reserved218[360]; /* 0x0218 */
+ __u64 pv_grregs[16]; /* 0x380 */
+ __u8 reserved400[512];
struct kvm_s390_itdb itdb; /* 0x0600 */
__u8 reserved700[2304]; /* 0x0700 */
};
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 490fde080107..97d3a81e5074 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3965,6 +3965,7 @@ static int vcpu_post_run(struct kvm_vcpu *vcpu, int exit_reason)
static int __vcpu_run(struct kvm_vcpu *vcpu)
{
int rc, exit_reason;
+ struct sie_page *sie_page = (struct sie_page *)vcpu->arch.sie_block;
/*
* We try to hold kvm->srcu during most of vcpu_run (except when run-
@@ -3986,8 +3987,18 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
guest_enter_irqoff();
__disable_cpu_timer_accounting(vcpu);
local_irq_enable();
+ if (kvm_s390_pv_is_protected(vcpu->kvm)) {
+ memcpy(sie_page->pv_grregs,
+ vcpu->run->s.regs.gprs,
+ sizeof(sie_page->pv_grregs));
+ }
exit_reason = sie64a(vcpu->arch.sie_block,
vcpu->run->s.regs.gprs);
+ if (kvm_s390_pv_is_protected(vcpu->kvm)) {
+ memcpy(vcpu->run->s.regs.gprs,
+ sie_page->pv_grregs,
+ sizeof(sie_page->pv_grregs));
+ }
sie64a will load/save gprs 0-13 from to vcpu->run->s.regs.gprs.
I would have assume that this is not required for prot virt, because the HW has direct access via the sie block?
Yes, that is correct. The load/save in sie64a is not necessary for pv guests.
1. Would it make sense to have a specialized sie64a() (or a parameter, e.g., if you pass in NULL in r3), that optimizes this loading/saving? Eventually we can also optimize which host registers to save/restore then.
Having 2 kinds of sie64a seems not very nice for just saving a small number of cycles.
2. Avoid this copying here. We have to store the state to vcpu->run->s.regs.gprs when returning to user space and restore the state when coming from user space.
I like this proposal better than the first one and
It was actually an additional proposal :)
1. avoids unnecessary saving/loading/saving/restoring
2. avoids the two memcpy
Also, we access the GPRS from interception handlers, there we might use wrappers like
kvm_s390_set_gprs()
kvm_s390_get_gprs()
having register accessors might be useful anyway.
But I would like to defer that to a later point in time to keep the changes in here
minimal?
We can add a "TODO" comment in here so that we do not forget about this
for a future patch. Makes sense?
While it makes sense, I guess one could come up with a patch for 2. in
less than 30 minutes ... but yeah, whatever you prefer. ;)
Just to get it fully right we'd need to:
a. Synchronize registers into/from vcpu run in sync_regs/store_regs
b. Sprinkle get/set_gpr(int nr) over most of the files in arch/s390/kvm
That's your proposal?
Yes. Patch 1, factor out gprs access. Patch 2, avoid the memcpy by
fixing the gprs access functions and removing the memcpys. (both as
addons to this patch)
I guess that should be it ... but maybe we'll stumble over surprises :)
--
Thanks,
David / dhildenb
