kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] KVM/MIPS32: Fixes for Linux 3.10
@ 2013-05-13 20:21 Sanjay Lal
  2013-05-13 20:21 ` [PATCH 1/2] KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it is a user visible API Sanjay Lal
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Sanjay Lal @ 2013-05-13 20:21 UTC (permalink / raw)
  To: kvm; +Cc: linux-mips, gleb, mtosatti, ralf, Sanjay Lal

The following patch set fixes a couple of issues with KVM/MIPS32 in Linux 3.10.

- As suggested by Gleb, wrap calls to gfn_to_pfn() with srcu_read_lock/unlock().
- kvm_mips_map_page() now returns an error code to it's callers, instead of calling panic()
  if it cannot find a mapping for a particular gfn.
- Follow the latest convention and move the kvm.h API file under uapi/...

--
Sanjay Lal (2):
  KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it
    is a user visible API.
  KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()

 arch/mips/include/asm/kvm.h      | 55 ----------------------------------------
 arch/mips/include/uapi/asm/kvm.h | 55 ++++++++++++++++++++++++++++++++++++++++
 arch/mips/kvm/kvm_tlb.c          | 38 ++++++++++++++++++++-------
 3 files changed, 84 insertions(+), 64 deletions(-)
 delete mode 100644 arch/mips/include/asm/kvm.h
 create mode 100644 arch/mips/include/uapi/asm/kvm.h

-- 
1.7.11.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it is a user visible API.
  2013-05-13 20:21 [PATCH 0/2] KVM/MIPS32: Fixes for Linux 3.10 Sanjay Lal
@ 2013-05-13 20:21 ` Sanjay Lal
  2013-05-13 20:21 ` [PATCH 2/2] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock() Sanjay Lal
  2013-05-13 21:07 ` [PATCH 0/2] KVM/MIPS32: Fixes for Linux 3.10 David Daney
  2 siblings, 0 replies; 11+ messages in thread
From: Sanjay Lal @ 2013-05-13 20:21 UTC (permalink / raw)
  To: kvm; +Cc: linux-mips, gleb, mtosatti, ralf, Sanjay Lal


Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm.h      | 55 ----------------------------------------
 arch/mips/include/uapi/asm/kvm.h | 55 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 55 deletions(-)
 delete mode 100644 arch/mips/include/asm/kvm.h
 create mode 100644 arch/mips/include/uapi/asm/kvm.h

diff --git a/arch/mips/include/asm/kvm.h b/arch/mips/include/asm/kvm.h
deleted file mode 100644
index 85789ea..0000000
--- a/arch/mips/include/asm/kvm.h
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
-* This file is subject to the terms and conditions of the GNU General Public
-* License.  See the file "COPYING" in the main directory of this archive
-* for more details.
-*
-* Copyright (C) 2012  MIPS Technologies, Inc.  All rights reserved.
-* Authors: Sanjay Lal <sanjayl@kymasys.com>
-*/
-
-#ifndef __LINUX_KVM_MIPS_H
-#define __LINUX_KVM_MIPS_H
-
-#include <linux/types.h>
-
-#define __KVM_MIPS
-
-#define N_MIPS_COPROC_REGS      32
-#define N_MIPS_COPROC_SEL   	8
-
-/* for KVM_GET_REGS and KVM_SET_REGS */
-struct kvm_regs {
-	__u32 gprs[32];
-	__u32 hi;
-	__u32 lo;
-	__u32 pc;
-
-	__u32 cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
-};
-
-/* for KVM_GET_SREGS and KVM_SET_SREGS */
-struct kvm_sregs {
-};
-
-/* for KVM_GET_FPU and KVM_SET_FPU */
-struct kvm_fpu {
-};
-
-struct kvm_debug_exit_arch {
-};
-
-/* for KVM_SET_GUEST_DEBUG */
-struct kvm_guest_debug_arch {
-};
-
-struct kvm_mips_interrupt {
-	/* in */
-	__u32 cpu;
-	__u32 irq;
-};
-
-/* definition of registers in kvm_run */
-struct kvm_sync_regs {
-};
-
-#endif /* __LINUX_KVM_MIPS_H */
diff --git a/arch/mips/include/uapi/asm/kvm.h b/arch/mips/include/uapi/asm/kvm.h
new file mode 100644
index 0000000..85789ea
--- /dev/null
+++ b/arch/mips/include/uapi/asm/kvm.h
@@ -0,0 +1,55 @@
+/*
+* This file is subject to the terms and conditions of the GNU General Public
+* License.  See the file "COPYING" in the main directory of this archive
+* for more details.
+*
+* Copyright (C) 2012  MIPS Technologies, Inc.  All rights reserved.
+* Authors: Sanjay Lal <sanjayl@kymasys.com>
+*/
+
+#ifndef __LINUX_KVM_MIPS_H
+#define __LINUX_KVM_MIPS_H
+
+#include <linux/types.h>
+
+#define __KVM_MIPS
+
+#define N_MIPS_COPROC_REGS      32
+#define N_MIPS_COPROC_SEL   	8
+
+/* for KVM_GET_REGS and KVM_SET_REGS */
+struct kvm_regs {
+	__u32 gprs[32];
+	__u32 hi;
+	__u32 lo;
+	__u32 pc;
+
+	__u32 cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
+};
+
+/* for KVM_GET_SREGS and KVM_SET_SREGS */
+struct kvm_sregs {
+};
+
+/* for KVM_GET_FPU and KVM_SET_FPU */
+struct kvm_fpu {
+};
+
+struct kvm_debug_exit_arch {
+};
+
+/* for KVM_SET_GUEST_DEBUG */
+struct kvm_guest_debug_arch {
+};
+
+struct kvm_mips_interrupt {
+	/* in */
+	__u32 cpu;
+	__u32 irq;
+};
+
+/* definition of registers in kvm_run */
+struct kvm_sync_regs {
+};
+
+#endif /* __LINUX_KVM_MIPS_H */
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-13 20:21 [PATCH 0/2] KVM/MIPS32: Fixes for Linux 3.10 Sanjay Lal
  2013-05-13 20:21 ` [PATCH 1/2] KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it is a user visible API Sanjay Lal
@ 2013-05-13 20:21 ` Sanjay Lal
  2013-05-14  9:27   ` Gleb Natapov
  2013-05-13 21:07 ` [PATCH 0/2] KVM/MIPS32: Fixes for Linux 3.10 David Daney
  2 siblings, 1 reply; 11+ messages in thread
From: Sanjay Lal @ 2013-05-13 20:21 UTC (permalink / raw)
  To: kvm; +Cc: linux-mips, gleb, mtosatti, ralf, Sanjay Lal


Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kvm/kvm_tlb.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
index 89511a9..218075f 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -16,7 +16,10 @@
 #include <linux/mm.h>
 #include <linux/delay.h>
 #include <linux/module.h>
+#include <linux/bootmem.h>
 #include <linux/kvm_host.h>
+#include <linux/srcu.h>
+
 
 #include <asm/cpu.h>
 #include <asm/bootinfo.h>
@@ -36,6 +39,8 @@
 /* Use VZ EntryHi.EHINV to invalidate TLB entries */
 #define UNIQUE_ENTRYHI(idx) (CKSEG0 + ((idx) << (PAGE_SHIFT + 1)))
 
+EXPORT_SYMBOL(min_low_pfn);     /* defined by bootmem.c, but not exported by generic code */
+
 atomic_t kvm_mips_instance;
 EXPORT_SYMBOL(kvm_mips_instance);
 
@@ -169,21 +174,27 @@ void kvm_mips_dump_shadow_tlbs(struct kvm_vcpu *vcpu)
 	}
 }
 
-static void kvm_mips_map_page(struct kvm *kvm, gfn_t gfn)
+static int kvm_mips_map_page(struct kvm *kvm, gfn_t gfn)
 {
+	int srcu_idx, err = 0;
 	pfn_t pfn;
 
 	if (kvm->arch.guest_pmap[gfn] != KVM_INVALID_PAGE)
-		return;
+		return 0;
 
+        srcu_idx = srcu_read_lock(&kvm->srcu);
 	pfn = kvm_mips_gfn_to_pfn(kvm, gfn);
 
 	if (kvm_mips_is_error_pfn(pfn)) {
-		panic("Couldn't get pfn for gfn %#" PRIx64 "!\n", gfn);
+		kvm_err("Couldn't get pfn for gfn %#" PRIx64 "!\n", gfn);
+		err = -EFAULT;
+		goto out;
 	}
 
 	kvm->arch.guest_pmap[gfn] = pfn;
-	return;
+out:
+	srcu_read_unlock(&kvm->srcu, srcu_idx);
+	return err;
 }
 
 /* Translate guest KSEG0 addresses to Host PA */
@@ -207,7 +218,10 @@ unsigned long kvm_mips_translate_guest_kseg0_to_hpa(struct kvm_vcpu *vcpu,
 			gva);
 		return KVM_INVALID_PAGE;
 	}
-	kvm_mips_map_page(vcpu->kvm, gfn);
+
+	if (kvm_mips_map_page(vcpu->kvm, gfn) < 0)
+		return KVM_INVALID_ADDR;
+
 	return (kvm->arch.guest_pmap[gfn] << PAGE_SHIFT) + offset;
 }
 
@@ -310,8 +324,11 @@ int kvm_mips_handle_kseg0_tlb_fault(unsigned long badvaddr,
 	even = !(gfn & 0x1);
 	vaddr = badvaddr & (PAGE_MASK << 1);
 
-	kvm_mips_map_page(vcpu->kvm, gfn);
-	kvm_mips_map_page(vcpu->kvm, gfn ^ 0x1);
+	if (kvm_mips_map_page(vcpu->kvm, gfn) < 0)
+		return -1;
+
+	if (kvm_mips_map_page(vcpu->kvm, gfn ^ 0x1) < 0)
+		return -1;
 
 	if (even) {
 		pfn0 = kvm->arch.guest_pmap[gfn];
@@ -389,8 +406,11 @@ kvm_mips_handle_mapped_seg_tlb_fault(struct kvm_vcpu *vcpu,
 		pfn0 = 0;
 		pfn1 = 0;
 	} else {
-		kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT);
-		kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT);
+		if (kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT) < 0)
+			return -1;
+
+		if (kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT) < 0)
+			return -1;
 
 		pfn0 = kvm->arch.guest_pmap[mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT];
 		pfn1 = kvm->arch.guest_pmap[mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT];
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] KVM/MIPS32: Fixes for Linux 3.10
  2013-05-13 20:21 [PATCH 0/2] KVM/MIPS32: Fixes for Linux 3.10 Sanjay Lal
  2013-05-13 20:21 ` [PATCH 1/2] KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it is a user visible API Sanjay Lal
  2013-05-13 20:21 ` [PATCH 2/2] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock() Sanjay Lal
@ 2013-05-13 21:07 ` David Daney
  2013-05-13 22:36   ` Sanjay Lal
  2 siblings, 1 reply; 11+ messages in thread
From: David Daney @ 2013-05-13 21:07 UTC (permalink / raw)
  To: Sanjay Lal; +Cc: kvm, linux-mips, gleb, mtosatti, ralf

On 05/13/2013 01:21 PM, Sanjay Lal wrote:
> The following patch set fixes a couple of issues with KVM/MIPS32 in Linux 3.10.
>
> - As suggested by Gleb, wrap calls to gfn_to_pfn() with srcu_read_lock/unlock().
> - kvm_mips_map_page() now returns an error code to it's callers, instead of calling panic()
>    if it cannot find a mapping for a particular gfn.
> - Follow the latest convention and move the kvm.h API file under uapi/...
>

Sanjay,

Have you looked at:

http://www.linux-mips.org/archives/linux-mips/2013-05/msg00049.html

We should start working toward unifying the KVM interface.

David Daney.


> --
> Sanjay Lal (2):
>    KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it
>      is a user visible API.
>    KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
>
>   arch/mips/include/asm/kvm.h      | 55 ----------------------------------------
>   arch/mips/include/uapi/asm/kvm.h | 55 ++++++++++++++++++++++++++++++++++++++++
>   arch/mips/kvm/kvm_tlb.c          | 38 ++++++++++++++++++++-------
>   3 files changed, 84 insertions(+), 64 deletions(-)
>   delete mode 100644 arch/mips/include/asm/kvm.h
>   create mode 100644 arch/mips/include/uapi/asm/kvm.h
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] KVM/MIPS32: Fixes for Linux 3.10
  2013-05-13 21:07 ` [PATCH 0/2] KVM/MIPS32: Fixes for Linux 3.10 David Daney
@ 2013-05-13 22:36   ` Sanjay Lal
  0 siblings, 0 replies; 11+ messages in thread
From: Sanjay Lal @ 2013-05-13 22:36 UTC (permalink / raw)
  To: David Daney; +Cc: kvm, linux-mips, gleb, mtosatti, ralf


On May 13, 2013, at 2:07 PM, David Daney wrote:

> Sanjay,
> 
> Have you looked at:
> 
> http://www.linux-mips.org/archives/linux-mips/2013-05/msg00049.html
> 
> We should start working toward unifying the KVM interface.
> 
> David Daney.
> 

Hey David, I am currently in the process of integrating and testing the new ABI. I'll need  a couple of days.

Regards
Sanjay


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-13 20:21 ` [PATCH 2/2] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock() Sanjay Lal
@ 2013-05-14  9:27   ` Gleb Natapov
  2013-05-15 15:54     ` Sanjay Lal
  0 siblings, 1 reply; 11+ messages in thread
From: Gleb Natapov @ 2013-05-14  9:27 UTC (permalink / raw)
  To: Sanjay Lal; +Cc: kvm, linux-mips, mtosatti, ralf

On Mon, May 13, 2013 at 01:21:40PM -0700, Sanjay Lal wrote:
> 

Explain kvm_mips_map_page() changes here please. The text from the cover
letter does not go into git history.

> Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
> ---
>  arch/mips/kvm/kvm_tlb.c | 38 +++++++++++++++++++++++++++++---------
>  1 file changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
> index 89511a9..218075f 100644
> --- a/arch/mips/kvm/kvm_tlb.c
> +++ b/arch/mips/kvm/kvm_tlb.c
> @@ -16,7 +16,10 @@
>  #include <linux/mm.h>
>  #include <linux/delay.h>
>  #include <linux/module.h>
> +#include <linux/bootmem.h>
>  #include <linux/kvm_host.h>
> +#include <linux/srcu.h>
> +
>  
>  #include <asm/cpu.h>
>  #include <asm/bootinfo.h>
> @@ -36,6 +39,8 @@
>  /* Use VZ EntryHi.EHINV to invalidate TLB entries */
>  #define UNIQUE_ENTRYHI(idx) (CKSEG0 + ((idx) << (PAGE_SHIFT + 1)))
>  
> +EXPORT_SYMBOL(min_low_pfn);     /* defined by bootmem.c, but not exported by generic code */
> +
What you need this for? It is not used anywhere in this patch and by
mips/kvm code in general.

>  atomic_t kvm_mips_instance;
>  EXPORT_SYMBOL(kvm_mips_instance);
>  
> @@ -169,21 +174,27 @@ void kvm_mips_dump_shadow_tlbs(struct kvm_vcpu *vcpu)
>  	}
>  }
>  
> -static void kvm_mips_map_page(struct kvm *kvm, gfn_t gfn)
> +static int kvm_mips_map_page(struct kvm *kvm, gfn_t gfn)
>  {
> +	int srcu_idx, err = 0;
>  	pfn_t pfn;
>  
>  	if (kvm->arch.guest_pmap[gfn] != KVM_INVALID_PAGE)
> -		return;
> +		return 0;
>  
> +        srcu_idx = srcu_read_lock(&kvm->srcu);
Use tabs to indent.

>  	pfn = kvm_mips_gfn_to_pfn(kvm, gfn);
>  
>  	if (kvm_mips_is_error_pfn(pfn)) {
> -		panic("Couldn't get pfn for gfn %#" PRIx64 "!\n", gfn);
> +		kvm_err("Couldn't get pfn for gfn %#" PRIx64 "!\n", gfn);
> +		err = -EFAULT;
> +		goto out;
>  	}
>  
>  	kvm->arch.guest_pmap[gfn] = pfn;
> -	return;
> +out:
> +	srcu_read_unlock(&kvm->srcu, srcu_idx);
> +	return err;
>  }
>  
>  /* Translate guest KSEG0 addresses to Host PA */
> @@ -207,7 +218,10 @@ unsigned long kvm_mips_translate_guest_kseg0_to_hpa(struct kvm_vcpu *vcpu,
>  			gva);
>  		return KVM_INVALID_PAGE;
>  	}
> -	kvm_mips_map_page(vcpu->kvm, gfn);
> +
> +	if (kvm_mips_map_page(vcpu->kvm, gfn) < 0)
> +		return KVM_INVALID_ADDR;
> +
>  	return (kvm->arch.guest_pmap[gfn] << PAGE_SHIFT) + offset;
>  }
>  
> @@ -310,8 +324,11 @@ int kvm_mips_handle_kseg0_tlb_fault(unsigned long badvaddr,
>  	even = !(gfn & 0x1);
>  	vaddr = badvaddr & (PAGE_MASK << 1);
>  
> -	kvm_mips_map_page(vcpu->kvm, gfn);
> -	kvm_mips_map_page(vcpu->kvm, gfn ^ 0x1);
> +	if (kvm_mips_map_page(vcpu->kvm, gfn) < 0)
> +		return -1;
> +
> +	if (kvm_mips_map_page(vcpu->kvm, gfn ^ 0x1) < 0)
> +		return -1;
>  
>  	if (even) {
>  		pfn0 = kvm->arch.guest_pmap[gfn];
> @@ -389,8 +406,11 @@ kvm_mips_handle_mapped_seg_tlb_fault(struct kvm_vcpu *vcpu,
>  		pfn0 = 0;
>  		pfn1 = 0;
>  	} else {
> -		kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT);
> -		kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT);
> +		if (kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT) < 0)
> +			return -1;
> +
> +		if (kvm_mips_map_page(kvm, mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT) < 0)
> +			return -1;
>  
>  		pfn0 = kvm->arch.guest_pmap[mips3_tlbpfn_to_paddr(tlb->tlb_lo0) >> PAGE_SHIFT];
>  		pfn1 = kvm->arch.guest_pmap[mips3_tlbpfn_to_paddr(tlb->tlb_lo1) >> PAGE_SHIFT];
> -- 
> 1.7.11.3

--
			Gleb.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-14  9:27   ` Gleb Natapov
@ 2013-05-15 15:54     ` Sanjay Lal
  2013-05-15 16:54       ` David Daney
  0 siblings, 1 reply; 11+ messages in thread
From: Sanjay Lal @ 2013-05-15 15:54 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: kvm, linux-mips, mtosatti, ralf


On May 14, 2013, at 2:27 AM, Gleb Natapov wrote:

>> 
>> 
>> +EXPORT_SYMBOL(min_low_pfn);     /* defined by bootmem.c, but not exported by generic code */
>> +
> What you need this for? It is not used anywhere in this patch and by
> mips/kvm code in general.

I did some digging around myself, since the linker keeps complaining that it can't find min_low_pfn when compiling the KVM module.  It seems that it is indirectly pulled in by the cache management functions.


Regards
Sanjay



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-15 15:54     ` Sanjay Lal
@ 2013-05-15 16:54       ` David Daney
  2013-05-15 17:30         ` Gleb Natapov
  0 siblings, 1 reply; 11+ messages in thread
From: David Daney @ 2013-05-15 16:54 UTC (permalink / raw)
  To: Sanjay Lal; +Cc: Gleb Natapov, kvm, linux-mips, mtosatti, ralf

On 05/15/2013 08:54 AM, Sanjay Lal wrote:
>
> On May 14, 2013, at 2:27 AM, Gleb Natapov wrote:
>
>>>
>>>
>>> +EXPORT_SYMBOL(min_low_pfn);     /* defined by bootmem.c, but not exported by generic code */
>>> +
>> What you need this for? It is not used anywhere in this patch and by
>> mips/kvm code in general.
>
> I did some digging around myself, since the linker keeps complaining that it can't find min_low_pfn when compiling the KVM module.  It seems that it is indirectly pulled in by the cache management functions.
>

If it is really needed, then the export should probably be done at the 
site of the min_low_pfn definition, not in some random architecture file.

An alternative is to fix the cache management functions so they don't 
require the export.

David Daney

>
> Regards
> Sanjay
>
>
>
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-15 16:54       ` David Daney
@ 2013-05-15 17:30         ` Gleb Natapov
  2013-05-15 18:36           ` Sanjay Lal
  0 siblings, 1 reply; 11+ messages in thread
From: Gleb Natapov @ 2013-05-15 17:30 UTC (permalink / raw)
  To: David Daney; +Cc: Sanjay Lal, kvm, linux-mips, mtosatti, ralf

On Wed, May 15, 2013 at 09:54:24AM -0700, David Daney wrote:
> On 05/15/2013 08:54 AM, Sanjay Lal wrote:
> >
> >On May 14, 2013, at 2:27 AM, Gleb Natapov wrote:
> >
> >>>
> >>>
> >>>+EXPORT_SYMBOL(min_low_pfn);     /* defined by bootmem.c, but not exported by generic code */
> >>>+
> >>What you need this for? It is not used anywhere in this patch and by
> >>mips/kvm code in general.
> >
> >I did some digging around myself, since the linker keeps complaining that it can't find min_low_pfn when compiling the KVM module.  It seems that it is indirectly pulled in by the cache management functions.
> >
> 
> If it is really needed, then the export should probably be done at
> the site of the min_low_pfn definition, not in some random
> architecture file.
> 
Definitely. We cannot snick it here like that. Please drop it from this
patch.

> An alternative is to fix the cache management functions so they
> don't require the export.
> 
> David Daney
> 
> >
> >Regards
> >Sanjay
> >
> >
> >
> >
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-15 17:30         ` Gleb Natapov
@ 2013-05-15 18:36           ` Sanjay Lal
  2013-05-16  5:53             ` Gleb Natapov
  0 siblings, 1 reply; 11+ messages in thread
From: Sanjay Lal @ 2013-05-15 18:36 UTC (permalink / raw)
  To: Gleb Natapov
  Cc: David Daney, kvm, linux-mips, Marcelo Tosatti, Ralf Baechle,
	Tony Luck, Stephen Rothwell


On May 15, 2013, at 10:30 AM, Gleb Natapov wrote:

> On Wed, May 15, 2013 at 09:54:24AM -0700, David Daney wrote:
>> On 05/15/2013 08:54 AM, Sanjay Lal wrote:
>>> 
>>> On May 14, 2013, at 2:27 AM, Gleb Natapov wrote:
>>> 
>>>>> 
>>>>> 
>>>>> +EXPORT_SYMBOL(min_low_pfn);     /* defined by bootmem.c, but not exported by generic code */
>>>>> +
>>>> What you need this for? It is not used anywhere in this patch and by
>>>> mips/kvm code in general.
>>> 
>>> I did some digging around myself, since the linker keeps complaining that it can't find min_low_pfn when compiling the KVM module.  It seems that it is indirectly pulled in by the cache management functions.
>>> 
>> 
>> If it is really needed, then the export should probably be done at
>> the site of the min_low_pfn definition, not in some random
>> architecture file.
>> 
> Definitely. We cannot snick it here like that. Please drop it from this
> patch.
> 

I did export min_low_pfn where it was defined (in .../mm/bootmem.c) as part of the original patch set. It conflicted with the ia64/metag ports.  min_low_pfn is exported in arch/ia64/kernel/ia64_ksyms.c and in arch/metag/kernel/metag_ksyms.c.

There was some chatter about this when the KVM/MIPS code ended up in linux-next.  From what I can gather, the maintainers for the other architectures agreed that exporting this symbol in bootmem.c was fine and should flow from the MIPS tree.  I'll do that as part of v2 of the patch set.

Regards
Sanjay


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock()
  2013-05-15 18:36           ` Sanjay Lal
@ 2013-05-16  5:53             ` Gleb Natapov
  0 siblings, 0 replies; 11+ messages in thread
From: Gleb Natapov @ 2013-05-16  5:53 UTC (permalink / raw)
  To: Sanjay Lal
  Cc: David Daney, kvm, linux-mips, Marcelo Tosatti, Ralf Baechle,
	Tony Luck, Stephen Rothwell

On Wed, May 15, 2013 at 11:36:02AM -0700, Sanjay Lal wrote:
> 
> On May 15, 2013, at 10:30 AM, Gleb Natapov wrote:
> 
> > On Wed, May 15, 2013 at 09:54:24AM -0700, David Daney wrote:
> >> On 05/15/2013 08:54 AM, Sanjay Lal wrote:
> >>> 
> >>> On May 14, 2013, at 2:27 AM, Gleb Natapov wrote:
> >>> 
> >>>>> 
> >>>>> 
> >>>>> +EXPORT_SYMBOL(min_low_pfn);     /* defined by bootmem.c, but not exported by generic code */
> >>>>> +
> >>>> What you need this for? It is not used anywhere in this patch and by
> >>>> mips/kvm code in general.
> >>> 
> >>> I did some digging around myself, since the linker keeps complaining that it can't find min_low_pfn when compiling the KVM module.  It seems that it is indirectly pulled in by the cache management functions.
> >>> 
> >> 
> >> If it is really needed, then the export should probably be done at
> >> the site of the min_low_pfn definition, not in some random
> >> architecture file.
> >> 
> > Definitely. We cannot snick it here like that. Please drop it from this
> > patch.
> > 
> 
> I did export min_low_pfn where it was defined (in .../mm/bootmem.c) as part of the original patch set. It conflicted with the ia64/metag ports.  min_low_pfn is exported in arch/ia64/kernel/ia64_ksyms.c and in arch/metag/kernel/metag_ksyms.c.
> 
> There was some chatter about this when the KVM/MIPS code ended up in linux-next.  From what I can gather, the maintainers for the other architectures agreed that exporting this symbol in bootmem.c was fine and should flow from the MIPS tree.  I'll do that as part of v2 of the patch set.
> 
Make it a separate patch and send it to linux-kernel and
linux-mm@kvack.org and affected arch maintainers. Or you can add export
to arch/mips/kernel/mips_ksyms.c and ask Ralf to take it. I can take it
via kvm tree with Ralf's ack too. In the commit message have a good
explanation why it is needed please.

--
			Gleb.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-05-16  5:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-13 20:21 [PATCH 0/2] KVM/MIPS32: Fixes for Linux 3.10 Sanjay Lal
2013-05-13 20:21 ` [PATCH 1/2] KVM/MIPS32: Move include/asm/kvm.h => include/uapi/asm/kvm.h since it is a user visible API Sanjay Lal
2013-05-13 20:21 ` [PATCH 2/2] KVM/MIPS32: Wrap calls to gfn_to_pfn() with srcu_read_lock/unlock() Sanjay Lal
2013-05-14  9:27   ` Gleb Natapov
2013-05-15 15:54     ` Sanjay Lal
2013-05-15 16:54       ` David Daney
2013-05-15 17:30         ` Gleb Natapov
2013-05-15 18:36           ` Sanjay Lal
2013-05-16  5:53             ` Gleb Natapov
2013-05-13 21:07 ` [PATCH 0/2] KVM/MIPS32: Fixes for Linux 3.10 David Daney
2013-05-13 22:36   ` Sanjay Lal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).