Re: [PATCH 2/2] pseries: Correctly create ibm,segment-page-sizes property
> > +for (count = 0; count< PPC_PAGE_SIZES_MAX_SZ; count++) {
> > +if (sps->enc[count].page_shift == 0) {
> > +break;
> > +}
> > +}
> > +if ((p - prop)>= (maxcells - 3 - count * 2))
>
> Is this valid C? Can you substract one pointer from another and compare
> the result with an int?
Ugh ... yeah it's basic stuff ;-) the difference between two pointers is
an integer (there's even a ptrdiff_t nowadays no ?)
> > +break;
>
> Braces? Please run checkpatch :)
Ah missed that one.
> > +*(p++) = cpu_to_be32(sps->page_shift);
> > +*(p++) = cpu_to_be32(sps->slb_enc);
> > +*(p++) = cpu_to_be32(count);
> > +for (j = 0; j< count; j++) {
> > +*(p++) = cpu_to_be32(sps->enc[j].page_shift);
> > +*(p++) = cpu_to_be32(sps->enc[j].pte_enc);
> > +}
> > +}
> > +
> > +return (p - prop) * sizeof(uint32_t);
>
> I'd prefer a second integer counter "len" I think :). Pointer
> arithmentics always make me wary...
>
And a separate variable that might accidentally get out of sync makes
_me_ wary :-)
> > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> > index 77aa186..860711c 100644
> > --- a/target-ppc/kvm.c
> > +++ b/target-ppc/kvm.c
> > @@ -201,6 +201,7 @@ static void kvm_get_fallback_smmu_info(CPUPPCState *env,
> > if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_GET_PVINFO)) {
> > /* No flags */
> > info->flags = 0;
> > +info->slb_size = 64;
>
> Eh - this one belongs in the first patch, no?
Quite possibly, the split in 2 patches was done by David (I originally
did a single patch), so I'm not 100% sure why he put that there, I'll
have a closer look today.
> > /* Standard 4k base page size segment */
> > info->sps[0].page_shift = 12;
> > @@ -218,9 +219,15 @@ static void kvm_get_fallback_smmu_info(CPUPPCState
> > *env,
> >
> > /* HV KVM has backing store size restrictions */
> > info->flags = KVM_PPC_PAGE_SIZES_REAL;
> > +if (env->mmu_model == POWERPC_MMU_2_06) {
> > +info->slb_size = 32;
> > +} else {
> > +info->slb_size = 64;
> > +}
>
> This assumes that we're always using -cpu host. Is there any more
> reliable way of calculating the slb size? Otherwise maybe we should just
> error out in non-cpu-host cases for HV mode.
This is a fallback, it's good enough. I don't think there's such a thing
as non-cpu-host on HV anyway, at least for now (we should probably error
out elsewhere). In any case, even if the CPU is configured for backward
compat (which we don't support yet, though might using -cpu in the long
run), the SLB size so far has to be exactly the one implemented by the
host when using HV KVM.
So for anything we can work on today, the above will work.
> > -if (env->mmu_model& POWERPC_MMU_1TSEG)
> > -info->flags = KVM_PPC_1T_SEGMENTS;
> > +if (env->mmu_model& POWERPC_MMU_1TSEG) {
> > +info->flags |= KVM_PPC_1T_SEGMENTS;
> > +}
>
> Ahem :)
What's this ? Second patch adding the braces missing in the first one ?
Heh, ok, I'll fix that.
Cheers,
Ben.
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ppc64: Rudimentary Support for extra page sizes on server CPUs
On Thu, 2012-05-10 at 20:55 +0300, Avi Kivity wrote: > On 05/10/2012 08:49 PM, Alexander Graf wrote: > >> +#if defined(TARGET_PPC64) > >> +if (def->sps) > >> +memcpy(&env->sps, def->sps, sizeof(*def->sps)); > > > > > > I never know if *def->... would dereference def or the complete > > construct. > > 'man operator' > > > How about sizeof(env->sps)? > > How about > > env->sps = *def->sps; Yeah, latter looks better doesn't it ? :-) I'll change that when I respin. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] pseries: Correctly create ibm,segment-page-sizes property
On 04/27/2012 07:51 AM, Benjamin Herrenschmidt wrote:
The core tcg/kvm code for ppc64 now has at least the outline
capability to support pagesizes beyond the standard 4k and 16MB. The
CPUState is initialized with information advertising the available
pagesizes and their correct encodings, and under the right KVM setup
this will be populated with page sizes beyond the standard.
Obviously guests can't use the extra page sizes unless they know
they're present. For the pseries machine, at least, there is a
defined method for conveying exactly this information, the
"ibm-segment-page-sizes" property in the guest device tree.
This patch generates this property using the supported page size
information that's already in the CPUState.
Signed-off-by: Nishanth Aravamudan
Signed-off-by: David Gibson
Signed-off-by: Benjamin Herrenschmidt
---
hw/spapr.c | 42 ++
target-ppc/kvm.c | 11 +--
2 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/hw/spapr.c b/hw/spapr.c
index 94a4e1e..7c36903 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -149,6 +149,39 @@ static int spapr_set_associativity(void *fdt,
sPAPREnvironment *spapr)
return ret;
}
+
+static size_t create_page_sizes_prop(CPUPPCState *env, uint32_t *prop,
+ size_t maxsize)
+{
+size_t maxcells = maxsize / sizeof(uint32_t);
+int i, j, count;
+uint32_t *p = prop;
+
+for (i = 0; i< PPC_PAGE_SIZES_MAX_SZ; i++) {
+struct ppc_one_seg_page_size *sps =&env->sps.sps[i];
+
+if (!sps->page_shift) {
+break;
+}
+for (count = 0; count< PPC_PAGE_SIZES_MAX_SZ; count++) {
+if (sps->enc[count].page_shift == 0) {
+break;
+}
+}
+if ((p - prop)>= (maxcells - 3 - count * 2))
Is this valid C? Can you substract one pointer from another and compare
the result with an int?
+break;
Braces? Please run checkpatch :)
+*(p++) = cpu_to_be32(sps->page_shift);
+*(p++) = cpu_to_be32(sps->slb_enc);
+*(p++) = cpu_to_be32(count);
+for (j = 0; j< count; j++) {
+*(p++) = cpu_to_be32(sps->enc[j].page_shift);
+*(p++) = cpu_to_be32(sps->enc[j].pte_enc);
+}
+}
+
+return (p - prop) * sizeof(uint32_t);
I'd prefer a second integer counter "len" I think :). Pointer
arithmentics always make me wary...
+}
+
static void *spapr_create_fdt_skel(const char *cpu_model,
target_phys_addr_t rma_size,
target_phys_addr_t initrd_base,
@@ -304,6 +337,8 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
0x, 0x};
uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ;
uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() :
10;
+uint32_t page_sizes_prop[64];
+size_t page_sizes_prop_size;
if ((index % smt) != 0) {
continue;
@@ -368,6 +403,13 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
_FDT((fdt_property_cell(fdt, "ibm,dfp", 1)));
}
+page_sizes_prop_size = create_page_sizes_prop(env, page_sizes_prop,
+ sizeof(page_sizes_prop));
+if (page_sizes_prop_size) {
+_FDT((fdt_property(fdt, "ibm,segment-page-sizes",
+ page_sizes_prop, page_sizes_prop_size)));
+}
+
_FDT((fdt_end_node(fdt)));
}
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 77aa186..860711c 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -201,6 +201,7 @@ static void kvm_get_fallback_smmu_info(CPUPPCState *env,
if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_GET_PVINFO)) {
/* No flags */
info->flags = 0;
+info->slb_size = 64;
Eh - this one belongs in the first patch, no?
/* Standard 4k base page size segment */
info->sps[0].page_shift = 12;
@@ -218,9 +219,15 @@ static void kvm_get_fallback_smmu_info(CPUPPCState *env,
/* HV KVM has backing store size restrictions */
info->flags = KVM_PPC_PAGE_SIZES_REAL;
+if (env->mmu_model == POWERPC_MMU_2_06) {
+info->slb_size = 32;
+} else {
+info->slb_size = 64;
+}
This assumes that we're always using -cpu host. Is there any more
reliable way of calculating the slb size? Otherwise maybe we should just
error out in non-cpu-host cases for HV mode.
-if (env->mmu_model& POWERPC_MMU_1TSEG)
-info->flags = KVM_PPC_1T_SEGMENTS;
+if (env->mmu_model& POWERPC_MMU_1TSEG) {
+info->flags |= KVM_PPC_1T_SEGMENTS;
+}
Ahem :)
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to major
Re: [PATCH 1/2] ppc64: Rudimentary Support for extra page sizes on server CPUs
On 05/10/2012 08:49 PM, Alexander Graf wrote: >> +#if defined(TARGET_PPC64) >> +if (def->sps) >> +memcpy(&env->sps, def->sps, sizeof(*def->sps)); > > > I never know if *def->... would dereference def or the complete > construct. 'man operator' > How about sizeof(env->sps)? How about env->sps = *def->sps; ? -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ppc64: Rudimentary Support for extra page sizes on server CPUs
On 04/27/2012 07:51 AM, Benjamin Herrenschmidt wrote:
More recent Power server chips (i.e. based on the 64 bit hash MMU)
support more than just the traditional 4k and 16M page sizes. This
can get quite complicated, because which page sizes are supported,
which combinations are supported within an MMU segment and how these
page sizes are encoded both in the SLB entry and the hash PTE can vary
depending on the CPU model (they are not specified by the
architecture). In addition the firmware or hypervisor may not permit
use of certain page sizes, for various reasons. Whether various page
sizes are supported on KVM, for example, depends on whether the PR or
HV variant of KVM is in use, and on the page size of the memory
backing the guest's RAM.
This patch adds information to the CPUState and cpu defs to describe
the supported page sizes and encodings. Since TCG does not yet
support any extended page sizes, we just set this to NULL in the
static CPU definitions, expanding this to the default 4k and 16M page
sizes when we initialize the cpu state. When using KVM, however, we
instead determine available page sizes using the new
KVM_PPC_GET_SMMU_INFO call. For old kernels without that call, we use
some defaults, with some guesswork which should do the right thing for
existing HV and PR implementations. The fallback might not be correct
for future versions, but that's ok, because they'll have
KVM_PPC_GET_SMMU_INFO.
Signed-off-by: Benjamin Herrenschmidt
Signed-off-by: David Gibson
---
This patch, to compile, requires a linux-headers/linux/kvm.h which
has the new capabilities and ioctl's defined (ie, from the kernel
side patch I posted earlier). This means getting the numbers locked
down with Avi
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 84c9674..d5891e4 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -874,6 +874,29 @@ enum {
#define DBELL_PIRTAG_MASK 0x3fff
/*/
+/* Segment page size information, used by recent hash MMUs
+ * The format of this structure mirrors kvm_ppc_smmu_info
+ */
+
+#define PPC_PAGE_SIZES_MAX_SZ 8
+
+struct ppc_one_page_size {
+uint32_t page_shift; /* Page shift (or 0) */
+uint32_t pte_enc;/* Encoding in the HPTE (>>12) */
+};
+
+struct ppc_one_seg_page_size {
+uint32_t page_shift; /* Base page shift of segment (or 0) */
+uint32_t slb_enc; /* SLB encoding for BookS */
+struct ppc_one_page_size enc[PPC_PAGE_SIZES_MAX_SZ];
+};
+
+struct ppc_segment_page_sizes {
+struct ppc_one_seg_page_size sps[PPC_PAGE_SIZES_MAX_SZ];
+};
+
+
+/*/
/* The whole PowerPC CPU context */
#define NB_MMU_MODES 3
@@ -889,6 +912,9 @@ struct ppc_def_t {
powerpc_input_t bus_model;
uint32_t flags;
int bfd_mach;
+#if defined(TARGET_PPC64)
+const struct ppc_segment_page_sizes *sps;
+#endif
void (*init_proc)(CPUPPCState *env);
int (*check_pow)(CPUPPCState *env);
};
@@ -1012,6 +1038,9 @@ struct CPUPPCState {
uint32_t flags;
uint64_t insns_flags;
uint64_t insns_flags2;
+#if defined(TARGET_PPC64)
+struct ppc_segment_page_sizes sps;
+#endif
#if defined(TARGET_PPC64)&& !defined(CONFIG_USER_ONLY)
target_phys_addr_t vpa;
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 0ab7630..77aa186 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
@@ -167,10 +168,208 @@ static int kvm_booke206_tlb_init(CPUPPCState *env)
return 0;
}
+
+#if defined (TARGET_PPC64)
+static void kvm_get_fallback_smmu_info(CPUPPCState *env,
+ struct kvm_ppc_smmu_info *info)
+{
+memset(info, 0, sizeof(*info));
+
+/* We don't have the new KVM_PPC_GET_SMMU_INFO ioctl, so
+ * need to "guess" what the supported page sizes are.
+ *
+ * For that to work we make a few assumptions:
+ *
+ * - If KVM_CAP_PPC_GET_PVINFO is supported we are running "PR"
+ * KVM which only supports 4K and 16M pages, but supports them
+ * regardless of the backing store characteritics. We also don't
+ * support 1T segments.
+ *
+ * This is safe as if HV KVM ever supports that capability or PR
+ * KVM grows supports for more page/segment sizes, those versions
+ * will have implemented KVM_CAP_PPC_GET_SMMU_INFO and thus we
+ * will not hit this fallback
+ *
+ * - Else we are running HV KVM. This means we only support page
+ * sizes that fit in the backing store. Additionally we only
+ * advertize 64K pages if the processor is ARCH 2.06 and we assume
+ * P7 encodings for the SLB and hash table. Here too, we assume
+ * support for any newer processor will mean a kernel that
+ * implements KVM_CAP_PPC_GET_SMMU_INFO and thus doesn't hit
+ * this fallback.
+
[PATCH] KVM: PPC: Book3S HV: Fix bug leading to deadlock in guest HPT updates
When handling the H_BULK_REMOVE hypercall, we were forgetting to
invalidate and unlock the hashed page table entry (HPTE) in the case
where the page had been paged out. This fixes it by clearing the
first doubleword of the HPTE in that case.
This fixes a regression introduced in commit a92bce95f0 ("KVM: PPC:
Book3S HV: Keep HPTE locked when invalidating"). The effect of the
regression is that the host kernel will sometimes hang when under
memory pressure.
Signed-off-by: Paul Mackerras
---
arch/powerpc/kvm/book3s_hv_rm_mmu.c |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index def880a..cec4dad 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
@@ -463,6 +463,7 @@ long kvmppc_h_bulk_remove(struct kvm_vcpu *vcpu)
/* insert R and C bits from PTE */
rcbits = rev->guest_rpte & (HPTE_R_R|HPTE_R_C);
args[j] |= rcbits << (56 - 5);
+ hp[0] = 0;
continue;
}
--
1.7.10.rc3.219.g53414
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
