> -----Original Message----- > From: kvm-ppc-owner@xxxxxxxxxxxxxxx > [mailto:kvm-ppc-owner@xxxxxxxxxxxxxxx] On Behalf Of Hollis Blanchard > Sent: Thursday, December 04, 2008 7:50 AM > To: Liu Yu-B13201 > Cc: kvm-ppc@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 1/3] kvmppc: Some refactor for merging e500 > > On Wed, 2008-12-03 at 17:28 +0800, Liu Yu wrote: > > Signed-off-by: Liu Yu <yu.liu@xxxxxxxxxxxxx> > > > > --- > > arch/powerpc/include/asm/kvm_44x.h | 9 +++++ > > arch/powerpc/include/asm/kvm_host.h | 7 ---- > > arch/powerpc/include/asm/kvm_ppc.h | 5 +-- > > arch/powerpc/kvm/44x.c | 22 +++++++++++++- > > arch/powerpc/kvm/44x_tlb.c | 29 +++++------------ > > arch/powerpc/kvm/44x_tlb.h | 19 +++++++++-- > > arch/powerpc/kvm/booke.c | 49 > +++++++++---------------------- > > arch/powerpc/kvm/booke.h | 8 +++++ > > arch/powerpc/kvm/booke_interrupts.S | 56 > ++++------------------------------ > > arch/powerpc/kvm/booke_interrupts.h | 54 > +++++++++++++++++++++++++++++++++ > > 10 files changed, 139 insertions(+), 119 deletions(-) > > create mode 100644 arch/powerpc/kvm/booke_interrupts.h > > This patch already doesn't apply. (I know it's awkward that I > don't have > a published git tree, and I'm sorry about that.) Aside from that, the > patch is also a lot to review at once. Splitting it into smaller > logically separate patches would ease both problems. > > For example, I see at least the following patches here: > * moving struct kvmppc_44x_tlbe > * kvmppc_booke_init() changes > * LOAD/SAVE_PID > * moving #include "44x_tlb.h" > * kvmppc_mmu_map() -> kvmppc_core_mmu_map() > * LOAD_GUEST_TLB -- this one is obsolete, so if it were > a separate > patch, we could just drop it and the rest would still apply > > Some of these I disagree with, so we need to be able to discuss them > individually. Here are some of my comments right now: > > I'm torn about the "map" changes because I wanted that function to be > generic (e.g. could apply for hash MMUs too), but it already > wasn't very > generic, so we can refactor later as needed. OK > > It looks like Linux (the host) never touches PID1 or PID2 > after boot. In > that case, we don't need to save/restore them in our exit handlers. Right. We don't need them for now. Then there is no need to add booke_intrrupts.h. > > I'm not sure the kvmppc_booke_init() refactoring is worth it. Can > kvmppc_e500_init() just memcpy the extra interrupt handlers itself? > There are only a few. Reasonable. The 16 exceptions are defined by booke, so the code should belong to booke. OK. I'll copy extra interrupt in kvmppc_e500_init(). -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
