On Thu, 13 Feb 2014 15:53:16 +0100
Alexander Graf <ag...@suse.de> wrote:
>
> On 13.02.2014, at 04:00, Aneesh Kumar K.V
> <aneesh.ku...@linux.vnet.ibm.com> wrote:
>
> > Greg Kurz <gk...@linux.vnet.ibm.com> writes:
> >
> >> The 396bb9874 commit reworked page table lookup to support kvm.
> >> Unfortunately this breaks 32 bit build:
> >>
> >> target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte0’:
> >> target-ppc/mmu-hash64.h:90:23: error: cast to pointer from integer of
> >> different size
> >>
> >> target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte1’:
> >> target-ppc/mmu-hash64.h:101:23: error: cast to pointer from integer of
> >> different size
> >>
> >> The problem is that we have two cases to handle here:
> >> - the HTAB is external and it is accessed with a pointer
> >> - the HTAB is emulated and it is accessed with a hwaddr
> >>
> >> Depending on the way the HTAB is controlled, we should use the
> >> appropriate type:
> >> - controlled by kvm, it is copied to an allocated buffer (pointer)
> >> - controlled by qemu with an allocated buffer (pointer)
> >> - controlled by qemu with soft mmu (hwaddr)
> >>
> >> This patch introduces an explicit distinction between the two cases in
> >> the new page table lookup code.
> >
> > Reviewed-by: Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com>
>
> I wouldn't mind something slightly lighter weight. How about this one
> instead? If you guys think it's better to have an actual type for the
> token I'd pull in this patch as is though.
>
>
> Alex
>
I confess... I consider castings evil and favor explicit typing. :)
On the other hand, I have no strong opinions against your patch. The "token"
code is quite simple and risks of confusion are low, we can live with it.
Please add:
Reviewed-by: Greg Kurz <gk...@linux.vnet.ibm.com>
Thanks for your time.
--
Greg
> diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> index 4e574d9..3240427 100644
> --- a/target-ppc/mmu-hash64.c
> +++ b/target-ppc/mmu-hash64.c
> @@ -340,7 +340,7 @@ uint64_t ppc_hash64_start_access(PowerPCCPU *cpu,
> target_ulong pte_index)
> * accessible PTEG.
> */
> if (cpu->env.external_htab) {
> - token = (uint64_t) cpu->env.external_htab + pte_offset;
> + token = (uint64_t)(uintptr_t) cpu->env.external_htab +
> pte_offset; } else if (cpu->env.htab_base) {
> token = cpu->env.htab_base + pte_offset;
> }
> diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
> index 9c9ca1d..8fb2ae4 100644
> --- a/target-ppc/mmu-hash64.h
> +++ b/target-ppc/mmu-hash64.h
> @@ -85,22 +85,24 @@ void ppc_hash64_stop_access(uint64_t token);
> static inline target_ulong ppc_hash64_load_hpte0(CPUPPCState *env,
> uint64_t token, int
> index) {
> - index *= HASH_PTE_SIZE_64;
> + uint64_t addr;
> + addr = token + (index * HASH_PTE_SIZE_64) + HASH_PTE_SIZE_64/2;
> if (env->external_htab) {
> - return ldq_p((const void *)(token + index));
> + return ldq_p((const void *)(uintptr_t)addr);
> } else {
> - return ldq_phys(token + index);
> + return ldq_phys(addr);
> }
> }
>
> static inline target_ulong ppc_hash64_load_hpte1(CPUPPCState *env,
> uint64_t token, int
> index) {
> - index *= HASH_PTE_SIZE_64;
> + uint64_t addr;
> + addr = token + (index * HASH_PTE_SIZE_64) + HASH_PTE_SIZE_64/2;
> if (env->external_htab) {
> - return ldq_p((const void *)(token + index +
> HASH_PTE_SIZE_64/2));
> + return ldq_p((const void *)(uintptr_t)addr);
> } else {
> - return ldq_phys(token + index + HASH_PTE_SIZE_64/2);
> + return ldq_phys(addr);
> }
> }
>
>
--
Gregory Kurz kurzg...@fr.ibm.com
gk...@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys http://www.ibm.com
Tel +33 (0)562 165 496
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
