* [PATCH 1/8] s390/kvm,gaccess: fix guest access return code handling
2013-03-05 12:14 [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups Christian Borntraeger
@ 2013-03-05 12:14 ` Christian Borntraeger
2013-03-05 12:14 ` [PATCH 2/8] s390/mm,gmap: implement gmap_translate() Christian Borntraeger
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2013-03-05 12:14 UTC (permalink / raw)
To: Marcelo Tossati, Gleb Natapov
Cc: Cornelia Huck, Heiko Carstens, Martin Schwidefsky, KVM,
linux-s390, Christian Borntraeger
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Guest access functions like copy_to/from_guest() call __guestaddr_to_user()
which in turn call gmap_fault() in order to translate a guest address to a
user space address.
In error case __guest_addr_to_user() returns either -EFAULT or -ENOMEM.
The copy_to/from_guest functions just pass these return values down to the
callers.
The -ENOMEM case however is problematic since there are several places
which access guest memory like:
rc = copy_to_guest(...);
if (rc == -EFAULT)
error_handling();
So in case of -ENOMEM the code assumes that the guest memory access
succeeded even though it failed.
This can cause guest data or state corruption.
If __guestaddr_to_user() returns -ENOMEM the meaning is that a valid user
space mapping exists, but there was not enough memory available when trying
to build the guest mapping. In other words an out-of-memory situation
occured.
For normal user space accesses an out-of-memory situation causes the page
fault handler to map -ENOMEM to -EFAULT (see fixup code in do_no_context()).
We need to do exactly the same for the kvm gaccess functions.
So __guestaddr_to_user() should just map all error codes to -EFAULT.
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/s390/kvm/gaccess.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
index 4703f12..84d01dd 100644
--- a/arch/s390/kvm/gaccess.h
+++ b/arch/s390/kvm/gaccess.h
@@ -22,13 +22,16 @@ static inline void __user *__guestaddr_to_user(struct kvm_vcpu *vcpu,
unsigned long guestaddr)
{
unsigned long prefix = vcpu->arch.sie_block->prefix;
+ unsigned long uaddress;
if (guestaddr < 2 * PAGE_SIZE)
guestaddr += prefix;
else if ((guestaddr >= prefix) && (guestaddr < prefix + 2 * PAGE_SIZE))
guestaddr -= prefix;
-
- return (void __user *) gmap_fault(guestaddr, vcpu->arch.gmap);
+ uaddress = gmap_fault(guestaddr, vcpu->arch.gmap);
+ if (IS_ERR_VALUE(uaddress))
+ uaddress = -EFAULT;
+ return (void __user *)uaddress;
}
static inline int get_guest_u64(struct kvm_vcpu *vcpu, unsigned long guestaddr,
--
1.8.0.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/8] s390/mm,gmap: implement gmap_translate()
2013-03-05 12:14 [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups Christian Borntraeger
2013-03-05 12:14 ` [PATCH 1/8] s390/kvm,gaccess: fix guest access return code handling Christian Borntraeger
@ 2013-03-05 12:14 ` Christian Borntraeger
2013-03-05 12:14 ` [PATCH 3/8] s390/kvm,tprot: use new gmap_translate() function Christian Borntraeger
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2013-03-05 12:14 UTC (permalink / raw)
To: Marcelo Tossati, Gleb Natapov
Cc: Cornelia Huck, Heiko Carstens, Martin Schwidefsky, KVM,
linux-s390, Christian Borntraeger
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Implement gmap_translate() function which translates a guest absolute address
to a user space process address without establishing the guest page table
entries.
This is useful for kvm guest address translations where no memory access
is expected to happen soon (e.g. tprot exception handler).
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/s390/include/asm/pgtable.h | 2 +
arch/s390/mm/pgtable.c | 107 +++++++++++++++++++++++++++++++---------
2 files changed, 87 insertions(+), 22 deletions(-)
diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
index 4a29308..75b8750 100644
--- a/arch/s390/include/asm/pgtable.h
+++ b/arch/s390/include/asm/pgtable.h
@@ -759,6 +759,8 @@ void gmap_disable(struct gmap *gmap);
int gmap_map_segment(struct gmap *gmap, unsigned long from,
unsigned long to, unsigned long length);
int gmap_unmap_segment(struct gmap *gmap, unsigned long to, unsigned long len);
+unsigned long __gmap_translate(unsigned long address, struct gmap *);
+unsigned long gmap_translate(unsigned long address, struct gmap *);
unsigned long __gmap_fault(unsigned long address, struct gmap *);
unsigned long gmap_fault(unsigned long address, struct gmap *);
void gmap_discard(unsigned long from, unsigned long to, struct gmap *);
diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
index ae44d2a..2accf71 100644
--- a/arch/s390/mm/pgtable.c
+++ b/arch/s390/mm/pgtable.c
@@ -379,45 +379,108 @@ out_unmap:
}
EXPORT_SYMBOL_GPL(gmap_map_segment);
+static unsigned long *gmap_table_walk(unsigned long address, struct gmap *gmap)
+{
+ unsigned long *table;
+
+ table = gmap->table + ((address >> 53) & 0x7ff);
+ if (unlikely(*table & _REGION_ENTRY_INV))
+ return ERR_PTR(-EFAULT);
+ table = (unsigned long *)(*table & _REGION_ENTRY_ORIGIN);
+ table = table + ((address >> 42) & 0x7ff);
+ if (unlikely(*table & _REGION_ENTRY_INV))
+ return ERR_PTR(-EFAULT);
+ table = (unsigned long *)(*table & _REGION_ENTRY_ORIGIN);
+ table = table + ((address >> 31) & 0x7ff);
+ if (unlikely(*table & _REGION_ENTRY_INV))
+ return ERR_PTR(-EFAULT);
+ table = (unsigned long *)(*table & _REGION_ENTRY_ORIGIN);
+ table = table + ((address >> 20) & 0x7ff);
+ return table;
+}
+
+/**
+ * __gmap_translate - translate a guest address to a user space address
+ * @address: guest address
+ * @gmap: pointer to guest mapping meta data structure
+ *
+ * Returns user space address which corresponds to the guest address or
+ * -EFAULT if no such mapping exists.
+ * This function does not establish potentially missing page table entries.
+ * The mmap_sem of the mm that belongs to the address space must be held
+ * when this function gets called.
+ */
+unsigned long __gmap_translate(unsigned long address, struct gmap *gmap)
+{
+ unsigned long *segment_ptr, vmaddr, segment;
+ struct gmap_pgtable *mp;
+ struct page *page;
+
+ current->thread.gmap_addr = address;
+ segment_ptr = gmap_table_walk(address, gmap);
+ if (IS_ERR(segment_ptr))
+ return PTR_ERR(segment_ptr);
+ /* Convert the gmap address to an mm address. */
+ segment = *segment_ptr;
+ if (!(segment & _SEGMENT_ENTRY_INV)) {
+ page = pfn_to_page(segment >> PAGE_SHIFT);
+ mp = (struct gmap_pgtable *) page->index;
+ return mp->vmaddr | (address & ~PMD_MASK);
+ } else if (segment & _SEGMENT_ENTRY_RO) {
+ vmaddr = segment & _SEGMENT_ENTRY_ORIGIN;
+ return vmaddr | (address & ~PMD_MASK);
+ }
+ return -EFAULT;
+}
+EXPORT_SYMBOL_GPL(__gmap_translate);
+
+/**
+ * gmap_translate - translate a guest address to a user space address
+ * @address: guest address
+ * @gmap: pointer to guest mapping meta data structure
+ *
+ * Returns user space address which corresponds to the guest address or
+ * -EFAULT if no such mapping exists.
+ * This function does not establish potentially missing page table entries.
+ */
+unsigned long gmap_translate(unsigned long address, struct gmap *gmap)
+{
+ unsigned long rc;
+
+ down_read(&gmap->mm->mmap_sem);
+ rc = __gmap_translate(address, gmap);
+ up_read(&gmap->mm->mmap_sem);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(gmap_translate);
+
/*
* this function is assumed to be called with mmap_sem held
*/
unsigned long __gmap_fault(unsigned long address, struct gmap *gmap)
{
- unsigned long *table, vmaddr, segment;
- struct mm_struct *mm;
+ unsigned long *segment_ptr, vmaddr, segment;
+ struct vm_area_struct *vma;
struct gmap_pgtable *mp;
struct gmap_rmap *rmap;
- struct vm_area_struct *vma;
+ struct mm_struct *mm;
struct page *page;
pgd_t *pgd;
pud_t *pud;
pmd_t *pmd;
current->thread.gmap_addr = address;
- mm = gmap->mm;
- /* Walk the gmap address space page table */
- table = gmap->table + ((address >> 53) & 0x7ff);
- if (unlikely(*table & _REGION_ENTRY_INV))
- return -EFAULT;
- table = (unsigned long *)(*table & _REGION_ENTRY_ORIGIN);
- table = table + ((address >> 42) & 0x7ff);
- if (unlikely(*table & _REGION_ENTRY_INV))
+ segment_ptr = gmap_table_walk(address, gmap);
+ if (IS_ERR(segment_ptr))
return -EFAULT;
- table = (unsigned long *)(*table & _REGION_ENTRY_ORIGIN);
- table = table + ((address >> 31) & 0x7ff);
- if (unlikely(*table & _REGION_ENTRY_INV))
- return -EFAULT;
- table = (unsigned long *)(*table & _REGION_ENTRY_ORIGIN);
- table = table + ((address >> 20) & 0x7ff);
-
/* Convert the gmap address to an mm address. */
- segment = *table;
- if (likely(!(segment & _SEGMENT_ENTRY_INV))) {
+ segment = *segment_ptr;
+ if (!(segment & _SEGMENT_ENTRY_INV)) {
page = pfn_to_page(segment >> PAGE_SHIFT);
mp = (struct gmap_pgtable *) page->index;
return mp->vmaddr | (address & ~PMD_MASK);
} else if (segment & _SEGMENT_ENTRY_RO) {
+ mm = gmap->mm;
vmaddr = segment & _SEGMENT_ENTRY_ORIGIN;
vma = find_vma(mm, vmaddr);
if (!vma || vma->vm_start > vmaddr)
@@ -441,12 +504,12 @@ unsigned long __gmap_fault(unsigned long address, struct gmap *gmap)
/* Link gmap segment table entry location to page table. */
page = pmd_page(*pmd);
mp = (struct gmap_pgtable *) page->index;
- rmap->entry = table;
+ rmap->entry = segment_ptr;
spin_lock(&mm->page_table_lock);
list_add(&rmap->list, &mp->mapper);
spin_unlock(&mm->page_table_lock);
/* Set gmap segment table entry to page table. */
- *table = pmd_val(*pmd) & PAGE_MASK;
+ *segment_ptr = pmd_val(*pmd) & PAGE_MASK;
return vmaddr | (address & ~PMD_MASK);
}
return -EFAULT;
--
1.8.0.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/8] s390/kvm,tprot: use new gmap_translate() function
2013-03-05 12:14 [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups Christian Borntraeger
2013-03-05 12:14 ` [PATCH 1/8] s390/kvm,gaccess: fix guest access return code handling Christian Borntraeger
2013-03-05 12:14 ` [PATCH 2/8] s390/mm,gmap: implement gmap_translate() Christian Borntraeger
@ 2013-03-05 12:14 ` Christian Borntraeger
2013-03-05 12:14 ` [PATCH 4/8] s390/kvm: remove explicit -EFAULT return code checking on guest access Christian Borntraeger
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2013-03-05 12:14 UTC (permalink / raw)
To: Marcelo Tossati, Gleb Natapov
Cc: Cornelia Huck, Heiko Carstens, Martin Schwidefsky, KVM,
linux-s390, Christian Borntraeger
From: Heiko Carstens <heiko.carstens@de.ibm.com>
When out-of-memory the tprot code incorrectly injected a program check
for the guest which reported an addressing exception even if the guest
address was valid.
Let's use the new gmap_translate() which translates a guest address to
a user space address whithout the chance of running into an out-of-memory
situation.
Also make it more explicit that for -EFAULT we won't find a vma.
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/s390/kvm/priv.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index 0ef9894..75ad91e 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -575,20 +575,13 @@ static int handle_tprot(struct kvm_vcpu *vcpu)
if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_DAT)
return -EOPNOTSUPP;
-
- /* we must resolve the address without holding the mmap semaphore.
- * This is ok since the userspace hypervisor is not supposed to change
- * the mapping while the guest queries the memory. Otherwise the guest
- * might crash or get wrong info anyway. */
- user_address = (unsigned long) __guestaddr_to_user(vcpu, address1);
-
down_read(¤t->mm->mmap_sem);
+ user_address = __gmap_translate(address1, vcpu->arch.gmap);
+ if (IS_ERR_VALUE(user_address))
+ goto out_inject;
vma = find_vma(current->mm, user_address);
- if (!vma) {
- up_read(¤t->mm->mmap_sem);
- return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
- }
-
+ if (!vma)
+ goto out_inject;
vcpu->arch.sie_block->gpsw.mask &= ~(3ul << 44);
if (!(vma->vm_flags & VM_WRITE) && (vma->vm_flags & VM_READ))
vcpu->arch.sie_block->gpsw.mask |= (1ul << 44);
@@ -597,6 +590,10 @@ static int handle_tprot(struct kvm_vcpu *vcpu)
up_read(¤t->mm->mmap_sem);
return 0;
+
+out_inject:
+ up_read(¤t->mm->mmap_sem);
+ return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
}
int kvm_s390_handle_e5(struct kvm_vcpu *vcpu)
--
1.8.0.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/8] s390/kvm: remove explicit -EFAULT return code checking on guest access
2013-03-05 12:14 [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups Christian Borntraeger
` (2 preceding siblings ...)
2013-03-05 12:14 ` [PATCH 3/8] s390/kvm,tprot: use new gmap_translate() function Christian Borntraeger
@ 2013-03-05 12:14 ` Christian Borntraeger
2013-03-05 12:14 ` [PATCH 5/8] s390/kvm,gaccess: shorten put/get_guest code Christian Borntraeger
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2013-03-05 12:14 UTC (permalink / raw)
To: Marcelo Tossati, Gleb Natapov
Cc: Cornelia Huck, Heiko Carstens, Martin Schwidefsky, KVM,
linux-s390, Christian Borntraeger
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Let's change to the paradigm that every return code from guest memory
access functions that is not zero translates to -EFAULT and do not
explictly compare.
Explictly comparing the return value with -EFAULT has already shown to
be a bit fragile. In addition this is closer to the handling of
copy_to/from_user functions, which imho is in general a good idea.
Also shorten the return code handling in interrupt.c a bit.
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/s390/kvm/intercept.c | 4 +-
arch/s390/kvm/interrupt.c | 241 +++++++++++++---------------------------------
arch/s390/kvm/priv.c | 6 +-
3 files changed, 74 insertions(+), 177 deletions(-)
diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
index f26ff1e..9b22047 100644
--- a/arch/s390/kvm/intercept.c
+++ b/arch/s390/kvm/intercept.c
@@ -45,7 +45,7 @@ static int handle_lctlg(struct kvm_vcpu *vcpu)
do {
rc = get_guest_u64(vcpu, useraddr,
&vcpu->arch.sie_block->gcr[reg]);
- if (rc == -EFAULT) {
+ if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
break;
}
@@ -79,7 +79,7 @@ static int handle_lctl(struct kvm_vcpu *vcpu)
reg = reg1;
do {
rc = get_guest_u32(vcpu, useraddr, &val);
- if (rc == -EFAULT) {
+ if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
break;
}
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 37116a7..5afa931 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -180,7 +180,7 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
struct kvm_s390_interrupt_info *inti)
{
const unsigned short table[] = { 2, 4, 4, 6 };
- int rc, exception = 0;
+ int rc = 0;
switch (inti->type) {
case KVM_S390_INT_EMERGENCY:
@@ -188,74 +188,38 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_emergency_signal++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->emerg.code, 0);
- rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x1201);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = put_guest_u16(vcpu, __LC_EXT_CPU_ADDR, inti->emerg.code);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
- &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
- __LC_EXT_NEW_PSW, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
+ rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x1201);
+ rc |= put_guest_u16(vcpu, __LC_EXT_CPU_ADDR, inti->emerg.code);
+ rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
+ &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
+ rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
+ __LC_EXT_NEW_PSW, sizeof(psw_t));
break;
-
case KVM_S390_INT_EXTERNAL_CALL:
VCPU_EVENT(vcpu, 4, "%s", "interrupt: sigp ext call");
vcpu->stat.deliver_external_call++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->extcall.code, 0);
- rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x1202);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = put_guest_u16(vcpu, __LC_EXT_CPU_ADDR, inti->extcall.code);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
- &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
- __LC_EXT_NEW_PSW, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
+ rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x1202);
+ rc |= put_guest_u16(vcpu, __LC_EXT_CPU_ADDR, inti->extcall.code);
+ rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
+ &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
+ rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
+ __LC_EXT_NEW_PSW, sizeof(psw_t));
break;
-
case KVM_S390_INT_SERVICE:
VCPU_EVENT(vcpu, 4, "interrupt: sclp parm:%x",
inti->ext.ext_params);
vcpu->stat.deliver_service_signal++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->ext.ext_params, 0);
- rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x2401);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
- &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
- __LC_EXT_NEW_PSW, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
-
- rc = put_guest_u32(vcpu, __LC_EXT_PARAMS, inti->ext.ext_params);
- if (rc == -EFAULT)
- exception = 1;
+ rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x2401);
+ rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
+ &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
+ rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
+ __LC_EXT_NEW_PSW, sizeof(psw_t));
+ rc |= put_guest_u32(vcpu, __LC_EXT_PARAMS, inti->ext.ext_params);
break;
-
case KVM_S390_INT_VIRTIO:
VCPU_EVENT(vcpu, 4, "interrupt: virtio parm:%x,parm64:%llx",
inti->ext.ext_params, inti->ext.ext_params2);
@@ -263,34 +227,16 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->ext.ext_params,
inti->ext.ext_params2);
- rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x2603);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = put_guest_u16(vcpu, __LC_EXT_CPU_ADDR, 0x0d00);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
- &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
- __LC_EXT_NEW_PSW, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
-
- rc = put_guest_u32(vcpu, __LC_EXT_PARAMS, inti->ext.ext_params);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = put_guest_u64(vcpu, __LC_EXT_PARAMS2,
- inti->ext.ext_params2);
- if (rc == -EFAULT)
- exception = 1;
+ rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x2603);
+ rc |= put_guest_u16(vcpu, __LC_EXT_CPU_ADDR, 0x0d00);
+ rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
+ &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
+ rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
+ __LC_EXT_NEW_PSW, sizeof(psw_t));
+ rc |= put_guest_u32(vcpu, __LC_EXT_PARAMS, inti->ext.ext_params);
+ rc |= put_guest_u64(vcpu, __LC_EXT_PARAMS2,
+ inti->ext.ext_params2);
break;
-
case KVM_S390_SIGP_STOP:
VCPU_EVENT(vcpu, 4, "%s", "interrupt: cpu stop");
vcpu->stat.deliver_stop_signal++;
@@ -313,18 +259,14 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_restart_signal++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
0, 0);
- rc = copy_to_guest(vcpu, offsetof(struct _lowcore,
- restart_old_psw), &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
- offsetof(struct _lowcore, restart_psw), sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
+ rc = copy_to_guest(vcpu,
+ offsetof(struct _lowcore, restart_old_psw),
+ &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
+ rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
+ offsetof(struct _lowcore, restart_psw),
+ sizeof(psw_t));
atomic_clear_mask(CPUSTAT_STOPPED, &vcpu->arch.sie_block->cpuflags);
break;
-
case KVM_S390_PROGRAM_INT:
VCPU_EVENT(vcpu, 4, "interrupt: pgm check code:%x, ilc:%x",
inti->pgm.code,
@@ -332,24 +274,13 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_program_int++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->pgm.code, 0);
- rc = put_guest_u16(vcpu, __LC_PGM_INT_CODE, inti->pgm.code);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = put_guest_u16(vcpu, __LC_PGM_ILC,
- table[vcpu->arch.sie_block->ipa >> 14]);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_to_guest(vcpu, __LC_PGM_OLD_PSW,
- &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
- __LC_PGM_NEW_PSW, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
+ rc = put_guest_u16(vcpu, __LC_PGM_INT_CODE, inti->pgm.code);
+ rc |= put_guest_u16(vcpu, __LC_PGM_ILC,
+ table[vcpu->arch.sie_block->ipa >> 14]);
+ rc |= copy_to_guest(vcpu, __LC_PGM_OLD_PSW,
+ &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
+ rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
+ __LC_PGM_NEW_PSW, sizeof(psw_t));
break;
case KVM_S390_MCHK:
@@ -358,24 +289,13 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->mchk.cr14,
inti->mchk.mcic);
- rc = kvm_s390_vcpu_store_status(vcpu,
- KVM_S390_STORE_STATUS_PREFIXED);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = put_guest_u64(vcpu, __LC_MCCK_CODE, inti->mchk.mcic);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_to_guest(vcpu, __LC_MCK_OLD_PSW,
- &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
- __LC_MCK_NEW_PSW, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
+ rc = kvm_s390_vcpu_store_status(vcpu,
+ KVM_S390_STORE_STATUS_PREFIXED);
+ rc |= put_guest_u64(vcpu, __LC_MCCK_CODE, inti->mchk.mcic);
+ rc |= copy_to_guest(vcpu, __LC_MCK_OLD_PSW,
+ &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
+ rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
+ __LC_MCK_NEW_PSW, sizeof(psw_t));
break;
case KVM_S390_INT_IO_MIN...KVM_S390_INT_IO_MAX:
@@ -388,67 +308,44 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_io_int++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
param0, param1);
- rc = put_guest_u16(vcpu, __LC_SUBCHANNEL_ID,
- inti->io.subchannel_id);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = put_guest_u16(vcpu, __LC_SUBCHANNEL_NR,
- inti->io.subchannel_nr);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = put_guest_u32(vcpu, __LC_IO_INT_PARM,
- inti->io.io_int_parm);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = put_guest_u32(vcpu, __LC_IO_INT_WORD,
- inti->io.io_int_word);
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_to_guest(vcpu, __LC_IO_OLD_PSW,
- &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
-
- rc = copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
- __LC_IO_NEW_PSW, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
+ rc = put_guest_u16(vcpu, __LC_SUBCHANNEL_ID,
+ inti->io.subchannel_id);
+ rc |= put_guest_u16(vcpu, __LC_SUBCHANNEL_NR,
+ inti->io.subchannel_nr);
+ rc |= put_guest_u32(vcpu, __LC_IO_INT_PARM,
+ inti->io.io_int_parm);
+ rc |= put_guest_u32(vcpu, __LC_IO_INT_WORD,
+ inti->io.io_int_word);
+ rc |= copy_to_guest(vcpu, __LC_IO_OLD_PSW,
+ &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
+ rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
+ __LC_IO_NEW_PSW, sizeof(psw_t));
break;
}
default:
BUG();
}
- if (exception) {
+ if (rc) {
printk("kvm: The guest lowcore is not mapped during interrupt "
- "delivery, killing userspace\n");
+ "delivery, killing userspace\n");
do_exit(SIGKILL);
}
}
static int __try_deliver_ckc_interrupt(struct kvm_vcpu *vcpu)
{
- int rc, exception = 0;
+ int rc;
if (psw_extint_disabled(vcpu))
return 0;
if (!(vcpu->arch.sie_block->gcr[0] & 0x800ul))
return 0;
- rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x1004);
- if (rc == -EFAULT)
- exception = 1;
- rc = copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
- &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
- rc = copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
- __LC_EXT_NEW_PSW, sizeof(psw_t));
- if (rc == -EFAULT)
- exception = 1;
- if (exception) {
+ rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x1004);
+ rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
+ &vcpu->arch.sie_block->gpsw, sizeof(psw_t));
+ rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
+ __LC_EXT_NEW_PSW, sizeof(psw_t));
+ if (rc) {
printk("kvm: The guest lowcore is not mapped during interrupt "
"delivery, killing userspace\n");
do_exit(SIGKILL);
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index 75ad91e..34b42dc 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -108,7 +108,7 @@ static int handle_store_cpu_address(struct kvm_vcpu *vcpu)
}
rc = put_guest_u16(vcpu, useraddr, vcpu->vcpu_id);
- if (rc == -EFAULT) {
+ if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
goto out;
}
@@ -230,7 +230,7 @@ static int handle_stfl(struct kvm_vcpu *vcpu)
rc = copy_to_guest(vcpu, offsetof(struct _lowcore, stfl_fac_list),
&facility_list, sizeof(facility_list));
- if (rc == -EFAULT)
+ if (rc)
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
else {
VCPU_EVENT(vcpu, 5, "store facility list value %x",
@@ -348,7 +348,7 @@ static int handle_stidp(struct kvm_vcpu *vcpu)
}
rc = put_guest_u64(vcpu, operand2, vcpu->arch.stidp_data);
- if (rc == -EFAULT) {
+ if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
goto out;
}
--
1.8.0.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/8] s390/kvm,gaccess: shorten put/get_guest code
2013-03-05 12:14 [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups Christian Borntraeger
` (3 preceding siblings ...)
2013-03-05 12:14 ` [PATCH 4/8] s390/kvm: remove explicit -EFAULT return code checking on guest access Christian Borntraeger
@ 2013-03-05 12:14 ` Christian Borntraeger
2013-03-05 12:14 ` [PATCH 6/8] s390/kvm,gaccess: shorten copy_to/from_guest code Christian Borntraeger
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2013-03-05 12:14 UTC (permalink / raw)
To: Marcelo Tossati, Gleb Natapov
Cc: Cornelia Huck, Heiko Carstens, Martin Schwidefsky, KVM,
linux-s390, Christian Borntraeger
From: Heiko Carstens <heiko.carstens@de.ibm.com>
The put_guest_u*/get_guest_u* are nothing but wrappers for the regular
put_user/get_user uaccess functions. The only difference is that before
accessing user space the guest address must be translated to a user space
address.
Change the order of arguments for the guest access functions so they
match their uaccess parts. Also remove the u* suffix, so we simply
have put_guest/get_guest which will automatically use the right size
dependent on pointer type of the destination/source that now must be
correct.
In result the same behaviour as put_user/get_user except that accesses
must be aligned.
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/s390/kvm/gaccess.h | 153 ++++++++++++----------------------------------
arch/s390/kvm/intercept.c | 6 +-
arch/s390/kvm/interrupt.c | 52 ++++++++--------
arch/s390/kvm/priv.c | 22 +++----
4 files changed, 81 insertions(+), 152 deletions(-)
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
index 84d01dd..82f450e 100644
--- a/arch/s390/kvm/gaccess.h
+++ b/arch/s390/kvm/gaccess.h
@@ -18,122 +18,47 @@
#include <asm/uaccess.h>
#include "kvm-s390.h"
-static inline void __user *__guestaddr_to_user(struct kvm_vcpu *vcpu,
- unsigned long guestaddr)
+static inline void *__gptr_to_uptr(struct kvm_vcpu *vcpu, void *gptr)
{
unsigned long prefix = vcpu->arch.sie_block->prefix;
- unsigned long uaddress;
-
- if (guestaddr < 2 * PAGE_SIZE)
- guestaddr += prefix;
- else if ((guestaddr >= prefix) && (guestaddr < prefix + 2 * PAGE_SIZE))
- guestaddr -= prefix;
- uaddress = gmap_fault(guestaddr, vcpu->arch.gmap);
- if (IS_ERR_VALUE(uaddress))
- uaddress = -EFAULT;
- return (void __user *)uaddress;
-}
-
-static inline int get_guest_u64(struct kvm_vcpu *vcpu, unsigned long guestaddr,
- u64 *result)
-{
- void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
-
- BUG_ON(guestaddr & 7);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- return get_user(*result, (unsigned long __user *) uptr);
-}
-
-static inline int get_guest_u32(struct kvm_vcpu *vcpu, unsigned long guestaddr,
- u32 *result)
-{
- void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
-
- BUG_ON(guestaddr & 3);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- return get_user(*result, (u32 __user *) uptr);
-}
-
-static inline int get_guest_u16(struct kvm_vcpu *vcpu, unsigned long guestaddr,
- u16 *result)
-{
- void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
-
- BUG_ON(guestaddr & 1);
-
- if (IS_ERR(uptr))
- return PTR_ERR(uptr);
-
- return get_user(*result, (u16 __user *) uptr);
-}
-
-static inline int get_guest_u8(struct kvm_vcpu *vcpu, unsigned long guestaddr,
- u8 *result)
-{
- void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- return get_user(*result, (u8 __user *) uptr);
-}
-
-static inline int put_guest_u64(struct kvm_vcpu *vcpu, unsigned long guestaddr,
- u64 value)
-{
- void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
-
- BUG_ON(guestaddr & 7);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- return put_user(value, (u64 __user *) uptr);
-}
-
-static inline int put_guest_u32(struct kvm_vcpu *vcpu, unsigned long guestaddr,
- u32 value)
-{
- void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
-
- BUG_ON(guestaddr & 3);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- return put_user(value, (u32 __user *) uptr);
-}
-
-static inline int put_guest_u16(struct kvm_vcpu *vcpu, unsigned long guestaddr,
- u16 value)
-{
- void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
-
- BUG_ON(guestaddr & 1);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- return put_user(value, (u16 __user *) uptr);
-}
-
-static inline int put_guest_u8(struct kvm_vcpu *vcpu, unsigned long guestaddr,
- u8 value)
-{
- void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- return put_user(value, (u8 __user *) uptr);
+ unsigned long gaddr = (unsigned long) gptr;
+ unsigned long uaddr;
+
+ if (gaddr < 2 * PAGE_SIZE)
+ gaddr += prefix;
+ else if ((gaddr >= prefix) && (gaddr < prefix + 2 * PAGE_SIZE))
+ gaddr -= prefix;
+ uaddr = gmap_fault(gaddr, vcpu->arch.gmap);
+ if (IS_ERR_VALUE(uaddr))
+ uaddr = -EFAULT;
+ return (void *)uaddr;
}
+#define get_guest(vcpu, x, gptr) \
+({ \
+ __typeof__(gptr) __uptr = __gptr_to_uptr(vcpu, gptr); \
+ int __mask = sizeof(__typeof__(*(gptr))) - 1; \
+ int __ret = PTR_RET(__uptr); \
+ \
+ if (!__ret) { \
+ BUG_ON((unsigned long)__uptr & __mask); \
+ __ret = get_user(x, __uptr); \
+ } \
+ __ret; \
+})
+
+#define put_guest(vcpu, x, gptr) \
+({ \
+ __typeof__(gptr) __uptr = __gptr_to_uptr(vcpu, gptr); \
+ int __mask = sizeof(__typeof__(*(gptr))) - 1; \
+ int __ret = PTR_RET(__uptr); \
+ \
+ if (!__ret) { \
+ BUG_ON((unsigned long)__uptr & __mask); \
+ __ret = put_user(x, __uptr); \
+ } \
+ __ret; \
+})
static inline int __copy_to_guest_slow(struct kvm_vcpu *vcpu,
unsigned long guestdest,
@@ -144,7 +69,7 @@ static inline int __copy_to_guest_slow(struct kvm_vcpu *vcpu,
u8 *data = from;
for (i = 0; i < n; i++) {
- rc = put_guest_u8(vcpu, guestdest++, *(data++));
+ rc = put_guest(vcpu, *(data++), (u8 *)guestdest++);
if (rc < 0)
return rc;
}
@@ -270,7 +195,7 @@ static inline int __copy_from_guest_slow(struct kvm_vcpu *vcpu, void *to,
u8 *data = to;
for (i = 0; i < n; i++) {
- rc = get_guest_u8(vcpu, guestsrc++, data++);
+ rc = get_guest(vcpu, *(data++), (u8 *)guestsrc++);
if (rc < 0)
return rc;
}
diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
index 9b22047..6474400 100644
--- a/arch/s390/kvm/intercept.c
+++ b/arch/s390/kvm/intercept.c
@@ -43,8 +43,8 @@ static int handle_lctlg(struct kvm_vcpu *vcpu)
trace_kvm_s390_handle_lctl(vcpu, 1, reg1, reg3, useraddr);
do {
- rc = get_guest_u64(vcpu, useraddr,
- &vcpu->arch.sie_block->gcr[reg]);
+ rc = get_guest(vcpu, vcpu->arch.sie_block->gcr[reg],
+ (u64 *) useraddr);
if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
break;
@@ -78,7 +78,7 @@ static int handle_lctl(struct kvm_vcpu *vcpu)
reg = reg1;
do {
- rc = get_guest_u32(vcpu, useraddr, &val);
+ rc = get_guest(vcpu, val, (u32 *) useraddr);
if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
break;
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 5afa931..d78824b 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -188,8 +188,9 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_emergency_signal++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->emerg.code, 0);
- rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x1201);
- rc |= put_guest_u16(vcpu, __LC_EXT_CPU_ADDR, inti->emerg.code);
+ rc = put_guest(vcpu, 0x1201, (u16 *)__LC_EXT_INT_CODE);
+ rc |= put_guest(vcpu, inti->emerg.code,
+ (u16 *)__LC_EXT_CPU_ADDR);
rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
@@ -200,8 +201,9 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_external_call++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->extcall.code, 0);
- rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x1202);
- rc |= put_guest_u16(vcpu, __LC_EXT_CPU_ADDR, inti->extcall.code);
+ rc = put_guest(vcpu, 0x1202, (u16 *)__LC_EXT_INT_CODE);
+ rc |= put_guest(vcpu, inti->extcall.code,
+ (u16 *)__LC_EXT_CPU_ADDR);
rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
@@ -213,12 +215,13 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_service_signal++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->ext.ext_params, 0);
- rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x2401);
+ rc = put_guest(vcpu, 0x2401, (u16 *)__LC_EXT_INT_CODE);
rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
__LC_EXT_NEW_PSW, sizeof(psw_t));
- rc |= put_guest_u32(vcpu, __LC_EXT_PARAMS, inti->ext.ext_params);
+ rc |= put_guest(vcpu, inti->ext.ext_params,
+ (u32 *)__LC_EXT_PARAMS);
break;
case KVM_S390_INT_VIRTIO:
VCPU_EVENT(vcpu, 4, "interrupt: virtio parm:%x,parm64:%llx",
@@ -227,15 +230,16 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->ext.ext_params,
inti->ext.ext_params2);
- rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x2603);
- rc |= put_guest_u16(vcpu, __LC_EXT_CPU_ADDR, 0x0d00);
+ rc = put_guest(vcpu, 0x2603, (u16 *)__LC_EXT_INT_CODE);
+ rc |= put_guest(vcpu, 0x0d00, (u16 *)__LC_EXT_CPU_ADDR);
rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
__LC_EXT_NEW_PSW, sizeof(psw_t));
- rc |= put_guest_u32(vcpu, __LC_EXT_PARAMS, inti->ext.ext_params);
- rc |= put_guest_u64(vcpu, __LC_EXT_PARAMS2,
- inti->ext.ext_params2);
+ rc |= put_guest(vcpu, inti->ext.ext_params,
+ (u32 *)__LC_EXT_PARAMS);
+ rc |= put_guest(vcpu, inti->ext.ext_params2,
+ (u64 *)__LC_EXT_PARAMS2);
break;
case KVM_S390_SIGP_STOP:
VCPU_EVENT(vcpu, 4, "%s", "interrupt: cpu stop");
@@ -274,9 +278,9 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_program_int++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->pgm.code, 0);
- rc = put_guest_u16(vcpu, __LC_PGM_INT_CODE, inti->pgm.code);
- rc |= put_guest_u16(vcpu, __LC_PGM_ILC,
- table[vcpu->arch.sie_block->ipa >> 14]);
+ rc = put_guest(vcpu, inti->pgm.code, (u16 *)__LC_PGM_INT_CODE);
+ rc |= put_guest(vcpu, table[vcpu->arch.sie_block->ipa >> 14],
+ (u16 *)__LC_PGM_ILC);
rc |= copy_to_guest(vcpu, __LC_PGM_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
@@ -291,7 +295,7 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
inti->mchk.mcic);
rc = kvm_s390_vcpu_store_status(vcpu,
KVM_S390_STORE_STATUS_PREFIXED);
- rc |= put_guest_u64(vcpu, __LC_MCCK_CODE, inti->mchk.mcic);
+ rc |= put_guest(vcpu, inti->mchk.mcic, (u64 *) __LC_MCCK_CODE);
rc |= copy_to_guest(vcpu, __LC_MCK_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
@@ -308,14 +312,14 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_io_int++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
param0, param1);
- rc = put_guest_u16(vcpu, __LC_SUBCHANNEL_ID,
- inti->io.subchannel_id);
- rc |= put_guest_u16(vcpu, __LC_SUBCHANNEL_NR,
- inti->io.subchannel_nr);
- rc |= put_guest_u32(vcpu, __LC_IO_INT_PARM,
- inti->io.io_int_parm);
- rc |= put_guest_u32(vcpu, __LC_IO_INT_WORD,
- inti->io.io_int_word);
+ rc = put_guest(vcpu, inti->io.subchannel_id,
+ (u16 *) __LC_SUBCHANNEL_ID);
+ rc |= put_guest(vcpu, inti->io.subchannel_nr,
+ (u16 *) __LC_SUBCHANNEL_NR);
+ rc |= put_guest(vcpu, inti->io.io_int_parm,
+ (u32 *) __LC_IO_INT_PARM);
+ rc |= put_guest(vcpu, inti->io.io_int_word,
+ (u32 *) __LC_IO_INT_WORD);
rc |= copy_to_guest(vcpu, __LC_IO_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
@@ -340,7 +344,7 @@ static int __try_deliver_ckc_interrupt(struct kvm_vcpu *vcpu)
return 0;
if (!(vcpu->arch.sie_block->gcr[0] & 0x800ul))
return 0;
- rc = put_guest_u16(vcpu, __LC_EXT_INT_CODE, 0x1004);
+ rc = put_guest(vcpu, 0x1004, (u16 *)__LC_EXT_INT_CODE);
rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index 34b42dc..cb07147 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -41,7 +41,7 @@ static int handle_set_prefix(struct kvm_vcpu *vcpu)
}
/* get the value */
- if (get_guest_u32(vcpu, operand2, &address)) {
+ if (get_guest(vcpu, address, (u32 *) operand2)) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
goto out;
}
@@ -82,7 +82,7 @@ static int handle_store_prefix(struct kvm_vcpu *vcpu)
address = address & 0x7fffe000u;
/* get the value */
- if (put_guest_u32(vcpu, operand2, address)) {
+ if (put_guest(vcpu, address, (u32 *)operand2)) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
goto out;
}
@@ -107,7 +107,7 @@ static int handle_store_cpu_address(struct kvm_vcpu *vcpu)
goto out;
}
- rc = put_guest_u16(vcpu, useraddr, vcpu->vcpu_id);
+ rc = put_guest(vcpu, vcpu->vcpu_id, (u16 *)useraddr);
if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
goto out;
@@ -142,18 +142,18 @@ static int handle_tpi(struct kvm_vcpu *vcpu)
* Store the two-word I/O interruption code into the
* provided area.
*/
- put_guest_u16(vcpu, addr, inti->io.subchannel_id);
- put_guest_u16(vcpu, addr + 2, inti->io.subchannel_nr);
- put_guest_u32(vcpu, addr + 4, inti->io.io_int_parm);
+ put_guest(vcpu, inti->io.subchannel_id, (u16 *) addr);
+ put_guest(vcpu, inti->io.subchannel_nr, (u16 *) (addr + 2));
+ put_guest(vcpu, inti->io.io_int_parm, (u32 *) (addr + 4));
} else {
/*
* Store the three-word I/O interruption code into
* the appropriate lowcore area.
*/
- put_guest_u16(vcpu, 184, inti->io.subchannel_id);
- put_guest_u16(vcpu, 186, inti->io.subchannel_nr);
- put_guest_u32(vcpu, 188, inti->io.io_int_parm);
- put_guest_u32(vcpu, 192, inti->io.io_int_word);
+ put_guest(vcpu, inti->io.subchannel_id, (u16 *) 184);
+ put_guest(vcpu, inti->io.subchannel_nr, (u16 *) 186);
+ put_guest(vcpu, inti->io.io_int_parm, (u32 *) 188);
+ put_guest(vcpu, inti->io.io_int_word, (u32 *) 192);
}
cc = 1;
} else
@@ -347,7 +347,7 @@ static int handle_stidp(struct kvm_vcpu *vcpu)
goto out;
}
- rc = put_guest_u64(vcpu, operand2, vcpu->arch.stidp_data);
+ rc = put_guest(vcpu, vcpu->arch.stidp_data, (u64 *)operand2);
if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
goto out;
--
1.8.0.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 6/8] s390/kvm,gaccess: shorten copy_to/from_guest code
2013-03-05 12:14 [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups Christian Borntraeger
` (4 preceding siblings ...)
2013-03-05 12:14 ` [PATCH 5/8] s390/kvm,gaccess: shorten put/get_guest code Christian Borntraeger
@ 2013-03-05 12:14 ` Christian Borntraeger
2013-03-05 12:14 ` [PATCH 7/8] s390/kvm: cleanup/fix handle_tpi() Christian Borntraeger
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2013-03-05 12:14 UTC (permalink / raw)
To: Marcelo Tossati, Gleb Natapov
Cc: Cornelia Huck, Heiko Carstens, Martin Schwidefsky, KVM,
linux-s390, Christian Borntraeger
From: Heiko Carstens <heiko.carstens@de.ibm.com>
The code can be significantly shortened. There is no functional change,
except that for large (> PAGE_SIZE) copies the guest translation would
be done more frequently.
However, there is not a single user which does this currently. If one
gets added later on this functionality can be added easily again.
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/s390/kvm/gaccess.h | 294 +++++++-----------------------------------------
1 file changed, 41 insertions(+), 253 deletions(-)
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
index 82f450e..8608d7e 100644
--- a/arch/s390/kvm/gaccess.h
+++ b/arch/s390/kvm/gaccess.h
@@ -18,16 +18,19 @@
#include <asm/uaccess.h>
#include "kvm-s390.h"
-static inline void *__gptr_to_uptr(struct kvm_vcpu *vcpu, void *gptr)
+static inline void *__gptr_to_uptr(struct kvm_vcpu *vcpu, void *gptr,
+ int prefixing)
{
unsigned long prefix = vcpu->arch.sie_block->prefix;
unsigned long gaddr = (unsigned long) gptr;
unsigned long uaddr;
- if (gaddr < 2 * PAGE_SIZE)
- gaddr += prefix;
- else if ((gaddr >= prefix) && (gaddr < prefix + 2 * PAGE_SIZE))
- gaddr -= prefix;
+ if (prefixing) {
+ if (gaddr < 2 * PAGE_SIZE)
+ gaddr += prefix;
+ else if ((gaddr >= prefix) && (gaddr < prefix + 2 * PAGE_SIZE))
+ gaddr -= prefix;
+ }
uaddr = gmap_fault(gaddr, vcpu->arch.gmap);
if (IS_ERR_VALUE(uaddr))
uaddr = -EFAULT;
@@ -36,7 +39,7 @@ static inline void *__gptr_to_uptr(struct kvm_vcpu *vcpu, void *gptr)
#define get_guest(vcpu, x, gptr) \
({ \
- __typeof__(gptr) __uptr = __gptr_to_uptr(vcpu, gptr); \
+ __typeof__(gptr) __uptr = __gptr_to_uptr(vcpu, gptr, 1);\
int __mask = sizeof(__typeof__(*(gptr))) - 1; \
int __ret = PTR_RET(__uptr); \
\
@@ -49,7 +52,7 @@ static inline void *__gptr_to_uptr(struct kvm_vcpu *vcpu, void *gptr)
#define put_guest(vcpu, x, gptr) \
({ \
- __typeof__(gptr) __uptr = __gptr_to_uptr(vcpu, gptr); \
+ __typeof__(gptr) __uptr = __gptr_to_uptr(vcpu, gptr, 1);\
int __mask = sizeof(__typeof__(*(gptr))) - 1; \
int __ret = PTR_RET(__uptr); \
\
@@ -60,255 +63,40 @@ static inline void *__gptr_to_uptr(struct kvm_vcpu *vcpu, void *gptr)
__ret; \
})
-static inline int __copy_to_guest_slow(struct kvm_vcpu *vcpu,
- unsigned long guestdest,
- void *from, unsigned long n)
-{
- int rc;
- unsigned long i;
- u8 *data = from;
-
- for (i = 0; i < n; i++) {
- rc = put_guest(vcpu, *(data++), (u8 *)guestdest++);
- if (rc < 0)
- return rc;
- }
- return 0;
-}
-
-static inline int __copy_to_guest_fast(struct kvm_vcpu *vcpu,
- unsigned long guestdest,
- void *from, unsigned long n)
-{
- int r;
- void __user *uptr;
- unsigned long size;
-
- if (guestdest + n < guestdest)
- return -EFAULT;
-
- /* simple case: all within one segment table entry? */
- if ((guestdest & PMD_MASK) == ((guestdest+n) & PMD_MASK)) {
- uptr = (void __user *) gmap_fault(guestdest, vcpu->arch.gmap);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- r = copy_to_user(uptr, from, n);
-
- if (r)
- r = -EFAULT;
-
- goto out;
- }
-
- /* copy first segment */
- uptr = (void __user *)gmap_fault(guestdest, vcpu->arch.gmap);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- size = PMD_SIZE - (guestdest & ~PMD_MASK);
-
- r = copy_to_user(uptr, from, size);
-
- if (r) {
- r = -EFAULT;
- goto out;
- }
- from += size;
- n -= size;
- guestdest += size;
-
- /* copy full segments */
- while (n >= PMD_SIZE) {
- uptr = (void __user *)gmap_fault(guestdest, vcpu->arch.gmap);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- r = copy_to_user(uptr, from, PMD_SIZE);
-
- if (r) {
- r = -EFAULT;
- goto out;
- }
- from += PMD_SIZE;
- n -= PMD_SIZE;
- guestdest += PMD_SIZE;
- }
-
- /* copy the tail segment */
- if (n) {
- uptr = (void __user *)gmap_fault(guestdest, vcpu->arch.gmap);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- r = copy_to_user(uptr, from, n);
-
- if (r)
- r = -EFAULT;
- }
-out:
- return r;
-}
-
-static inline int copy_to_guest_absolute(struct kvm_vcpu *vcpu,
- unsigned long guestdest,
- void *from, unsigned long n)
-{
- return __copy_to_guest_fast(vcpu, guestdest, from, n);
-}
-
-static inline int copy_to_guest(struct kvm_vcpu *vcpu, unsigned long guestdest,
- void *from, unsigned long n)
-{
- unsigned long prefix = vcpu->arch.sie_block->prefix;
-
- if ((guestdest < 2 * PAGE_SIZE) && (guestdest + n > 2 * PAGE_SIZE))
- goto slowpath;
-
- if ((guestdest < prefix) && (guestdest + n > prefix))
- goto slowpath;
-
- if ((guestdest < prefix + 2 * PAGE_SIZE)
- && (guestdest + n > prefix + 2 * PAGE_SIZE))
- goto slowpath;
-
- if (guestdest < 2 * PAGE_SIZE)
- guestdest += prefix;
- else if ((guestdest >= prefix) && (guestdest < prefix + 2 * PAGE_SIZE))
- guestdest -= prefix;
-
- return __copy_to_guest_fast(vcpu, guestdest, from, n);
-slowpath:
- return __copy_to_guest_slow(vcpu, guestdest, from, n);
-}
-
-static inline int __copy_from_guest_slow(struct kvm_vcpu *vcpu, void *to,
- unsigned long guestsrc,
- unsigned long n)
+static inline int __copy_guest(struct kvm_vcpu *vcpu, unsigned long to,
+ unsigned long from, unsigned long len,
+ int to_guest, int prefixing)
{
- int rc;
- unsigned long i;
- u8 *data = to;
-
- for (i = 0; i < n; i++) {
- rc = get_guest(vcpu, *(data++), (u8 *)guestsrc++);
- if (rc < 0)
- return rc;
+ unsigned long _len, rc;
+ void *uptr;
+
+ while (len) {
+ uptr = to_guest ? (void *)to : (void *)from;
+ uptr = __gptr_to_uptr(vcpu, uptr, prefixing);
+ if (IS_ERR(uptr))
+ return -EFAULT;
+ _len = PAGE_SIZE - ((unsigned long)uptr & (PAGE_SIZE - 1));
+ _len = min(_len, len);
+ if (to_guest)
+ rc = copy_to_user(uptr, (void *)from, _len);
+ else
+ rc = copy_from_user((void *)to, uptr, _len);
+ if (rc)
+ return -EFAULT;
+ len -= _len;
+ from += _len;
+ to += _len;
}
return 0;
}
-static inline int __copy_from_guest_fast(struct kvm_vcpu *vcpu, void *to,
- unsigned long guestsrc,
- unsigned long n)
-{
- int r;
- void __user *uptr;
- unsigned long size;
-
- if (guestsrc + n < guestsrc)
- return -EFAULT;
-
- /* simple case: all within one segment table entry? */
- if ((guestsrc & PMD_MASK) == ((guestsrc+n) & PMD_MASK)) {
- uptr = (void __user *) gmap_fault(guestsrc, vcpu->arch.gmap);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- r = copy_from_user(to, uptr, n);
-
- if (r)
- r = -EFAULT;
-
- goto out;
- }
-
- /* copy first segment */
- uptr = (void __user *)gmap_fault(guestsrc, vcpu->arch.gmap);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- size = PMD_SIZE - (guestsrc & ~PMD_MASK);
-
- r = copy_from_user(to, uptr, size);
-
- if (r) {
- r = -EFAULT;
- goto out;
- }
- to += size;
- n -= size;
- guestsrc += size;
-
- /* copy full segments */
- while (n >= PMD_SIZE) {
- uptr = (void __user *)gmap_fault(guestsrc, vcpu->arch.gmap);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
+#define copy_to_guest(vcpu, to, from, size) \
+ __copy_guest(vcpu, to, (unsigned long)from, size, 1, 1)
+#define copy_from_guest(vcpu, to, from, size) \
+ __copy_guest(vcpu, (unsigned long)to, from, size, 0, 1)
+#define copy_to_guest_absolute(vcpu, to, from, size) \
+ __copy_guest(vcpu, to, (unsigned long)from, size, 1, 0)
+#define copy_from_guest_absolute(vcpu, to, from, size) \
+ __copy_guest(vcpu, (unsigned long)to, from, size, 0, 0)
- r = copy_from_user(to, uptr, PMD_SIZE);
-
- if (r) {
- r = -EFAULT;
- goto out;
- }
- to += PMD_SIZE;
- n -= PMD_SIZE;
- guestsrc += PMD_SIZE;
- }
-
- /* copy the tail segment */
- if (n) {
- uptr = (void __user *)gmap_fault(guestsrc, vcpu->arch.gmap);
-
- if (IS_ERR((void __force *) uptr))
- return PTR_ERR((void __force *) uptr);
-
- r = copy_from_user(to, uptr, n);
-
- if (r)
- r = -EFAULT;
- }
-out:
- return r;
-}
-
-static inline int copy_from_guest_absolute(struct kvm_vcpu *vcpu, void *to,
- unsigned long guestsrc,
- unsigned long n)
-{
- return __copy_from_guest_fast(vcpu, to, guestsrc, n);
-}
-
-static inline int copy_from_guest(struct kvm_vcpu *vcpu, void *to,
- unsigned long guestsrc, unsigned long n)
-{
- unsigned long prefix = vcpu->arch.sie_block->prefix;
-
- if ((guestsrc < 2 * PAGE_SIZE) && (guestsrc + n > 2 * PAGE_SIZE))
- goto slowpath;
-
- if ((guestsrc < prefix) && (guestsrc + n > prefix))
- goto slowpath;
-
- if ((guestsrc < prefix + 2 * PAGE_SIZE)
- && (guestsrc + n > prefix + 2 * PAGE_SIZE))
- goto slowpath;
-
- if (guestsrc < 2 * PAGE_SIZE)
- guestsrc += prefix;
- else if ((guestsrc >= prefix) && (guestsrc < prefix + 2 * PAGE_SIZE))
- guestsrc -= prefix;
-
- return __copy_from_guest_fast(vcpu, to, guestsrc, n);
-slowpath:
- return __copy_from_guest_slow(vcpu, to, guestsrc, n);
-}
-#endif
+#endif /* __KVM_S390_GACCESS_H */
--
1.8.0.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 7/8] s390/kvm: cleanup/fix handle_tpi()
2013-03-05 12:14 [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups Christian Borntraeger
` (5 preceding siblings ...)
2013-03-05 12:14 ` [PATCH 6/8] s390/kvm,gaccess: shorten copy_to/from_guest code Christian Borntraeger
@ 2013-03-05 12:14 ` Christian Borntraeger
2013-03-05 12:14 ` [PATCH 8/8] s390/kvm,gaccess: add address space annotations Christian Borntraeger
2013-03-07 19:21 ` [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups Marcelo Tosatti
8 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2013-03-05 12:14 UTC (permalink / raw)
To: Marcelo Tossati, Gleb Natapov
Cc: Cornelia Huck, Heiko Carstens, Martin Schwidefsky, KVM,
linux-s390, Christian Borntraeger
From: Heiko Carstens <heiko.carstens@de.ibm.com>
- add missing specification exception check
- remove one level of indentation
- use defines instead of magic numbers
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/s390/kvm/priv.c | 54 +++++++++++++++++++++++++++++-----------------------
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index cb07147..d64382c 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -14,6 +14,7 @@
#include <linux/kvm.h>
#include <linux/gfp.h>
#include <linux/errno.h>
+#include <asm/asm-offsets.h>
#include <asm/current.h>
#include <asm/debug.h>
#include <asm/ebcdic.h>
@@ -129,39 +130,44 @@ static int handle_skey(struct kvm_vcpu *vcpu)
static int handle_tpi(struct kvm_vcpu *vcpu)
{
- u64 addr;
struct kvm_s390_interrupt_info *inti;
+ u64 addr;
int cc;
addr = kvm_s390_get_base_disp_s(vcpu);
-
+ if (addr & 3) {
+ kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
+ goto out;
+ }
+ cc = 0;
inti = kvm_s390_get_io_int(vcpu->kvm, vcpu->run->s.regs.crs[6], 0);
- if (inti) {
- if (addr) {
- /*
- * Store the two-word I/O interruption code into the
- * provided area.
- */
- put_guest(vcpu, inti->io.subchannel_id, (u16 *) addr);
- put_guest(vcpu, inti->io.subchannel_nr, (u16 *) (addr + 2));
- put_guest(vcpu, inti->io.io_int_parm, (u32 *) (addr + 4));
- } else {
- /*
- * Store the three-word I/O interruption code into
- * the appropriate lowcore area.
- */
- put_guest(vcpu, inti->io.subchannel_id, (u16 *) 184);
- put_guest(vcpu, inti->io.subchannel_nr, (u16 *) 186);
- put_guest(vcpu, inti->io.io_int_parm, (u32 *) 188);
- put_guest(vcpu, inti->io.io_int_word, (u32 *) 192);
- }
- cc = 1;
- } else
- cc = 0;
+ if (!inti)
+ goto no_interrupt;
+ cc = 1;
+ if (addr) {
+ /*
+ * Store the two-word I/O interruption code into the
+ * provided area.
+ */
+ put_guest(vcpu, inti->io.subchannel_id, (u16 *) addr);
+ put_guest(vcpu, inti->io.subchannel_nr, (u16 *) (addr + 2));
+ put_guest(vcpu, inti->io.io_int_parm, (u32 *) (addr + 4));
+ } else {
+ /*
+ * Store the three-word I/O interruption code into
+ * the appropriate lowcore area.
+ */
+ put_guest(vcpu, inti->io.subchannel_id, (u16 *) __LC_SUBCHANNEL_ID);
+ put_guest(vcpu, inti->io.subchannel_nr, (u16 *) __LC_SUBCHANNEL_NR);
+ put_guest(vcpu, inti->io.io_int_parm, (u32 *) __LC_IO_INT_PARM);
+ put_guest(vcpu, inti->io.io_int_word, (u32 *) __LC_IO_INT_WORD);
+ }
kfree(inti);
+no_interrupt:
/* Set condition code and we're done. */
vcpu->arch.sie_block->gpsw.mask &= ~(3ul << 44);
vcpu->arch.sie_block->gpsw.mask |= (cc & 3ul) << 44;
+out:
return 0;
}
--
1.8.0.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 8/8] s390/kvm,gaccess: add address space annotations
2013-03-05 12:14 [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups Christian Borntraeger
` (6 preceding siblings ...)
2013-03-05 12:14 ` [PATCH 7/8] s390/kvm: cleanup/fix handle_tpi() Christian Borntraeger
@ 2013-03-05 12:14 ` Christian Borntraeger
2013-03-07 19:21 ` [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups Marcelo Tosatti
8 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2013-03-05 12:14 UTC (permalink / raw)
To: Marcelo Tossati, Gleb Natapov
Cc: Cornelia Huck, Heiko Carstens, Martin Schwidefsky, KVM,
linux-s390, Christian Borntraeger
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Add missing address space annotations to all put_guest()/get_guest() callers.
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/s390/kvm/gaccess.h | 21 +++++++++++----------
arch/s390/kvm/intercept.c | 4 ++--
arch/s390/kvm/interrupt.c | 36 ++++++++++++++++++------------------
arch/s390/kvm/priv.c | 22 +++++++++++-----------
4 files changed, 42 insertions(+), 41 deletions(-)
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
index 8608d7e..302e0e5 100644
--- a/arch/s390/kvm/gaccess.h
+++ b/arch/s390/kvm/gaccess.h
@@ -18,8 +18,9 @@
#include <asm/uaccess.h>
#include "kvm-s390.h"
-static inline void *__gptr_to_uptr(struct kvm_vcpu *vcpu, void *gptr,
- int prefixing)
+static inline void __user *__gptr_to_uptr(struct kvm_vcpu *vcpu,
+ void __user *gptr,
+ int prefixing)
{
unsigned long prefix = vcpu->arch.sie_block->prefix;
unsigned long gaddr = (unsigned long) gptr;
@@ -34,14 +35,14 @@ static inline void *__gptr_to_uptr(struct kvm_vcpu *vcpu, void *gptr,
uaddr = gmap_fault(gaddr, vcpu->arch.gmap);
if (IS_ERR_VALUE(uaddr))
uaddr = -EFAULT;
- return (void *)uaddr;
+ return (void __user *)uaddr;
}
#define get_guest(vcpu, x, gptr) \
({ \
__typeof__(gptr) __uptr = __gptr_to_uptr(vcpu, gptr, 1);\
int __mask = sizeof(__typeof__(*(gptr))) - 1; \
- int __ret = PTR_RET(__uptr); \
+ int __ret = PTR_RET((void __force *)__uptr); \
\
if (!__ret) { \
BUG_ON((unsigned long)__uptr & __mask); \
@@ -54,7 +55,7 @@ static inline void *__gptr_to_uptr(struct kvm_vcpu *vcpu, void *gptr,
({ \
__typeof__(gptr) __uptr = __gptr_to_uptr(vcpu, gptr, 1);\
int __mask = sizeof(__typeof__(*(gptr))) - 1; \
- int __ret = PTR_RET(__uptr); \
+ int __ret = PTR_RET((void __force *)__uptr); \
\
if (!__ret) { \
BUG_ON((unsigned long)__uptr & __mask); \
@@ -68,19 +69,19 @@ static inline int __copy_guest(struct kvm_vcpu *vcpu, unsigned long to,
int to_guest, int prefixing)
{
unsigned long _len, rc;
- void *uptr;
+ void __user *uptr;
while (len) {
- uptr = to_guest ? (void *)to : (void *)from;
+ uptr = to_guest ? (void __user *)to : (void __user *)from;
uptr = __gptr_to_uptr(vcpu, uptr, prefixing);
- if (IS_ERR(uptr))
+ if (IS_ERR((void __force *)uptr))
return -EFAULT;
_len = PAGE_SIZE - ((unsigned long)uptr & (PAGE_SIZE - 1));
_len = min(_len, len);
if (to_guest)
- rc = copy_to_user(uptr, (void *)from, _len);
+ rc = copy_to_user((void __user *) uptr, (void *)from, _len);
else
- rc = copy_from_user((void *)to, uptr, _len);
+ rc = copy_from_user((void *)to, (void __user *)uptr, _len);
if (rc)
return -EFAULT;
len -= _len;
diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
index 6474400..c6ba4df 100644
--- a/arch/s390/kvm/intercept.c
+++ b/arch/s390/kvm/intercept.c
@@ -44,7 +44,7 @@ static int handle_lctlg(struct kvm_vcpu *vcpu)
do {
rc = get_guest(vcpu, vcpu->arch.sie_block->gcr[reg],
- (u64 *) useraddr);
+ (u64 __user *) useraddr);
if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
break;
@@ -78,7 +78,7 @@ static int handle_lctl(struct kvm_vcpu *vcpu)
reg = reg1;
do {
- rc = get_guest(vcpu, val, (u32 *) useraddr);
+ rc = get_guest(vcpu, val, (u32 __user *) useraddr);
if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
break;
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index d78824b..5c94817 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -188,9 +188,9 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_emergency_signal++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->emerg.code, 0);
- rc = put_guest(vcpu, 0x1201, (u16 *)__LC_EXT_INT_CODE);
+ rc = put_guest(vcpu, 0x1201, (u16 __user *)__LC_EXT_INT_CODE);
rc |= put_guest(vcpu, inti->emerg.code,
- (u16 *)__LC_EXT_CPU_ADDR);
+ (u16 __user *)__LC_EXT_CPU_ADDR);
rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
@@ -201,9 +201,9 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_external_call++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->extcall.code, 0);
- rc = put_guest(vcpu, 0x1202, (u16 *)__LC_EXT_INT_CODE);
+ rc = put_guest(vcpu, 0x1202, (u16 __user *)__LC_EXT_INT_CODE);
rc |= put_guest(vcpu, inti->extcall.code,
- (u16 *)__LC_EXT_CPU_ADDR);
+ (u16 __user *)__LC_EXT_CPU_ADDR);
rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
@@ -215,13 +215,13 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_service_signal++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->ext.ext_params, 0);
- rc = put_guest(vcpu, 0x2401, (u16 *)__LC_EXT_INT_CODE);
+ rc = put_guest(vcpu, 0x2401, (u16 __user *)__LC_EXT_INT_CODE);
rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
__LC_EXT_NEW_PSW, sizeof(psw_t));
rc |= put_guest(vcpu, inti->ext.ext_params,
- (u32 *)__LC_EXT_PARAMS);
+ (u32 __user *)__LC_EXT_PARAMS);
break;
case KVM_S390_INT_VIRTIO:
VCPU_EVENT(vcpu, 4, "interrupt: virtio parm:%x,parm64:%llx",
@@ -230,16 +230,16 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->ext.ext_params,
inti->ext.ext_params2);
- rc = put_guest(vcpu, 0x2603, (u16 *)__LC_EXT_INT_CODE);
- rc |= put_guest(vcpu, 0x0d00, (u16 *)__LC_EXT_CPU_ADDR);
+ rc = put_guest(vcpu, 0x2603, (u16 __user *)__LC_EXT_INT_CODE);
+ rc |= put_guest(vcpu, 0x0d00, (u16 __user *)__LC_EXT_CPU_ADDR);
rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
__LC_EXT_NEW_PSW, sizeof(psw_t));
rc |= put_guest(vcpu, inti->ext.ext_params,
- (u32 *)__LC_EXT_PARAMS);
+ (u32 __user *)__LC_EXT_PARAMS);
rc |= put_guest(vcpu, inti->ext.ext_params2,
- (u64 *)__LC_EXT_PARAMS2);
+ (u64 __user *)__LC_EXT_PARAMS2);
break;
case KVM_S390_SIGP_STOP:
VCPU_EVENT(vcpu, 4, "%s", "interrupt: cpu stop");
@@ -278,9 +278,9 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
vcpu->stat.deliver_program_int++;
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
inti->pgm.code, 0);
- rc = put_guest(vcpu, inti->pgm.code, (u16 *)__LC_PGM_INT_CODE);
+ rc = put_guest(vcpu, inti->pgm.code, (u16 __user *)__LC_PGM_INT_CODE);
rc |= put_guest(vcpu, table[vcpu->arch.sie_block->ipa >> 14],
- (u16 *)__LC_PGM_ILC);
+ (u16 __user *)__LC_PGM_ILC);
rc |= copy_to_guest(vcpu, __LC_PGM_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
@@ -295,7 +295,7 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
inti->mchk.mcic);
rc = kvm_s390_vcpu_store_status(vcpu,
KVM_S390_STORE_STATUS_PREFIXED);
- rc |= put_guest(vcpu, inti->mchk.mcic, (u64 *) __LC_MCCK_CODE);
+ rc |= put_guest(vcpu, inti->mchk.mcic, (u64 __user *) __LC_MCCK_CODE);
rc |= copy_to_guest(vcpu, __LC_MCK_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
@@ -313,13 +313,13 @@ static void __do_deliver_interrupt(struct kvm_vcpu *vcpu,
trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, inti->type,
param0, param1);
rc = put_guest(vcpu, inti->io.subchannel_id,
- (u16 *) __LC_SUBCHANNEL_ID);
+ (u16 __user *) __LC_SUBCHANNEL_ID);
rc |= put_guest(vcpu, inti->io.subchannel_nr,
- (u16 *) __LC_SUBCHANNEL_NR);
+ (u16 __user *) __LC_SUBCHANNEL_NR);
rc |= put_guest(vcpu, inti->io.io_int_parm,
- (u32 *) __LC_IO_INT_PARM);
+ (u32 __user *) __LC_IO_INT_PARM);
rc |= put_guest(vcpu, inti->io.io_int_word,
- (u32 *) __LC_IO_INT_WORD);
+ (u32 __user *) __LC_IO_INT_WORD);
rc |= copy_to_guest(vcpu, __LC_IO_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
@@ -344,7 +344,7 @@ static int __try_deliver_ckc_interrupt(struct kvm_vcpu *vcpu)
return 0;
if (!(vcpu->arch.sie_block->gcr[0] & 0x800ul))
return 0;
- rc = put_guest(vcpu, 0x1004, (u16 *)__LC_EXT_INT_CODE);
+ rc = put_guest(vcpu, 0x1004, (u16 __user *)__LC_EXT_INT_CODE);
rc |= copy_to_guest(vcpu, __LC_EXT_OLD_PSW,
&vcpu->arch.sie_block->gpsw, sizeof(psw_t));
rc |= copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw,
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index d64382c..7db2ad0 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -42,7 +42,7 @@ static int handle_set_prefix(struct kvm_vcpu *vcpu)
}
/* get the value */
- if (get_guest(vcpu, address, (u32 *) operand2)) {
+ if (get_guest(vcpu, address, (u32 __user *) operand2)) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
goto out;
}
@@ -83,7 +83,7 @@ static int handle_store_prefix(struct kvm_vcpu *vcpu)
address = address & 0x7fffe000u;
/* get the value */
- if (put_guest(vcpu, address, (u32 *)operand2)) {
+ if (put_guest(vcpu, address, (u32 __user *)operand2)) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
goto out;
}
@@ -108,7 +108,7 @@ static int handle_store_cpu_address(struct kvm_vcpu *vcpu)
goto out;
}
- rc = put_guest(vcpu, vcpu->vcpu_id, (u16 *)useraddr);
+ rc = put_guest(vcpu, vcpu->vcpu_id, (u16 __user *)useraddr);
if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
goto out;
@@ -149,18 +149,18 @@ static int handle_tpi(struct kvm_vcpu *vcpu)
* Store the two-word I/O interruption code into the
* provided area.
*/
- put_guest(vcpu, inti->io.subchannel_id, (u16 *) addr);
- put_guest(vcpu, inti->io.subchannel_nr, (u16 *) (addr + 2));
- put_guest(vcpu, inti->io.io_int_parm, (u32 *) (addr + 4));
+ put_guest(vcpu, inti->io.subchannel_id, (u16 __user *) addr);
+ put_guest(vcpu, inti->io.subchannel_nr, (u16 __user *) (addr + 2));
+ put_guest(vcpu, inti->io.io_int_parm, (u32 __user *) (addr + 4));
} else {
/*
* Store the three-word I/O interruption code into
* the appropriate lowcore area.
*/
- put_guest(vcpu, inti->io.subchannel_id, (u16 *) __LC_SUBCHANNEL_ID);
- put_guest(vcpu, inti->io.subchannel_nr, (u16 *) __LC_SUBCHANNEL_NR);
- put_guest(vcpu, inti->io.io_int_parm, (u32 *) __LC_IO_INT_PARM);
- put_guest(vcpu, inti->io.io_int_word, (u32 *) __LC_IO_INT_WORD);
+ put_guest(vcpu, inti->io.subchannel_id, (u16 __user *) __LC_SUBCHANNEL_ID);
+ put_guest(vcpu, inti->io.subchannel_nr, (u16 __user *) __LC_SUBCHANNEL_NR);
+ put_guest(vcpu, inti->io.io_int_parm, (u32 __user *) __LC_IO_INT_PARM);
+ put_guest(vcpu, inti->io.io_int_word, (u32 __user *) __LC_IO_INT_WORD);
}
kfree(inti);
no_interrupt:
@@ -353,7 +353,7 @@ static int handle_stidp(struct kvm_vcpu *vcpu)
goto out;
}
- rc = put_guest(vcpu, vcpu->arch.stidp_data, (u64 *)operand2);
+ rc = put_guest(vcpu, vcpu->arch.stidp_data, (u64 __user *)operand2);
if (rc) {
kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
goto out;
--
1.8.0.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups
2013-03-05 12:14 [PATCH 0/8] s390/kvm: memory mgmt related fixes/cleanups Christian Borntraeger
` (7 preceding siblings ...)
2013-03-05 12:14 ` [PATCH 8/8] s390/kvm,gaccess: add address space annotations Christian Borntraeger
@ 2013-03-07 19:21 ` Marcelo Tosatti
8 siblings, 0 replies; 10+ messages in thread
From: Marcelo Tosatti @ 2013-03-07 19:21 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Gleb Natapov, Cornelia Huck, Heiko Carstens, Martin Schwidefsky,
KVM, linux-s390
On Tue, Mar 05, 2013 at 01:14:39PM +0100, Christian Borntraeger wrote:
> Marcelo, Gleb,
>
> here are some patches to cleanup/fix some aspects of the KVM
> memory management on s390. Since these patches mostly touch
> files names */kvm*/ lets push them via the kvm tree.
> Please queue for 3.10.
>
> The patch queue applies on 3.9-rc1
Applied, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread