On Thu, Feb 23, 2017 at 04:02:54PM +1100, Alexey Kardashevskiy wrote: > On 23/02/17 13:09, David Gibson wrote: > > Accesses to the hashed page table (HPT) are complicated by the fact that > > the HPT could be in one of three places: > > 1) Within guest memory - when we're emulating a full guest CPU at the > > hardware level (e.g. powernv, mac99, g3beige) > > 2) Within qemu, but outside guest memory - when we're emulating user and > > supervisor instructions within TCG, but instead of emulating > > the CPU's hypervisor mode, we just emulate a hypervisor's behaviour > > (pseries in TCG) > > 3) Within KVM - a pseries machine using KVM acceleration. Mostly > > accesses to the HPT are handled by KVM, but there are a few cases > > where qemu needs to access it via a special fd for the purpose. > > > > In order to batch accesses to the fd in case (3), we use a somewhat awkward > > ppc_hash64_start_access() / ppc_hash64_stop_access() pair, which for case > > (3) reads / releases a whole PTEG from the kernel. For cases (1) & (2) > > it just returns an address value. The actual HPTE load helpers then need > > to interpret the returned token differently in the 3 cases. > > > > This patch keeps the same basic structure, but simplfiies the details. > > First start_access() / stop_access() are renamed to get_pteg() and > > put_pteg() to make their operation more obvious.
[snip]
> >
> > /*****************************************************************************/
> > -/* Types used to describe some PowerPC registers */
> > +/* Types used to describe some PowerPC registers etc. */
> > typedef struct DisasContext DisasContext;
> > typedef struct ppc_spr_t ppc_spr_t;
> > typedef union ppc_avr_t ppc_avr_t;
> > typedef union ppc_tlb_t ppc_tlb_t;
> > +typedef struct ppc_hash_pte64 ppc_hash_pte64_t;
> >
> > /* SPR access micro-ops generations callbacks */
> > struct ppc_spr_t {
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index 52bbea5..9d3e57e 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -2601,17 +2601,17 @@ struct kvm_get_htab_buf {
> > /*
> > * We require one extra byte for read
> > */
> > - target_ulong hpte[(HPTES_PER_GROUP * 2) + 1];
> > + ppc_hash_pte64_t hpte[HPTES_PER_GROUP];
>
>
> The "one extra byte" (which was ulong) is not needed any more why?
>
>
> > };
> >
> > -uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index)
> > +const ppc_hash_pte64_t *kvmppc_map_hptes(hwaddr ptex, int n)
>
>
> This "int n" is ignored here by a reason?
So looking at these comments, I realized the current code for reading
the HPTEs from KVM is just broken. So, for v2, I've prepended a patch
to fix that first, after which I don't need to touch the KVM side for
the rest of the series.
[snip]
> > +const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu,\
>
>
> You do not need the trailing '\'.
Missed this comment on my first pass, fixed now, thanks.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
