Re: [Qemu-devel] [RFC 0/3] Draft implementation of HPT resizing (qemu side)

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3916 bytes --]

On Fri, Jan 29, 2016 at 08:18:39AM +0200, Alexander Graf wrote:
> 
> 
> > Am 29.01.2016 um 04:47 schrieb David Gibson <david@gibson.dropbear.id.au>:
> > 
> >> On Thu, Jan 28, 2016 at 10:04:58PM +0100, Alexander Graf wrote:
> >> 
> >> 
> >>> On 01/19/2016 12:02 PM, David Gibson wrote:
> >>>> On Tue, Jan 19, 2016 at 01:18:17PM +0530, Bharata B Rao wrote:
> >>>>> On Mon, Jan 18, 2016 at 04:44:38PM +1100, David Gibson wrote:
> >>>>> Here is a draft qemu implementation of my proposed PAPR extension for
> >>>>> allowing runtime resizing of a KVM/ppc64 guest's hash page table.
> >>>>> That in turn will allow for more flexible memory hotplug.
> >>>>> 
> >>>>> This should work with the guest kernel side patches I also posted
> >>>>> recently [1].
> >>>>> 
> >>>>> Still required to make this into a full implementation:
> >>>>>  * Guest needs to auto-resize HPT on memory hotplug events
> >>>>> 
> >>>>>  * qemu needs to allocate HPT size based on current rather than
> >>>>>    maximum memory if the guest is HPT resize aware
> >>>>> 
> >>>>>  * KVM host side implementation
> >>>>> 
> >>>>>  * PAPR standardization
> >>>> So with the current patchset (QEMU and guest kernel changes), I should
> >>>> be able to change the HTAB size of a PR guest right ? I see the below
> >>>> failure though:
> >>> Uh.. to be honest I haven't really considered the KVM case at all.
> >>> I'm kind of surprised it didn't just refuse to do anything.
> >>> 
> >>>> [root@localhost ~]# cat /sys/kernel/debug/powerpc/pft-size
> >>>> 24
> >>>> [root@localhost ~]# echo 26 > /sys/kernel/debug/powerpc/pft-size
> >>>> [   65.996845] lpar: Attempting to resize HPT to shift 26
> >>>> [   65.996845] lpar: Attempting to resize HPT to shift 26
> >>>> [   66.113596] lpar: HPT resize to shift 26 complete (109 ms / 6 ms)
> >>>> [   66.113596] lpar: HPT resize to shift 26 complete (109 ms / 6 ms)
> >>>> 
> >>>> PR guest just hangs here while I see tons of below messages in
> >>>> the 1st level guest:
> >>>> 
> >>>> KVM can't copy data from 0x3fff99e91400!
> >>>> ...
> >>>> Couldn't emulate instruction 0x00000000 (op 0 xop 0)
> >>>> kvmppc_handle_exit_pr: emulation at 700 failed (00000000)
> >>> Hm, not sure why that's happening.  At first I thought it was because
> >>> we weren't updating SDR1 with the address of the new htab, but that's
> >>> actually in there.  Maybe the KVM PR code isn't rereading it after
> >>> initial VM startup.
> >> 
> >> The KVM PR code doesn't care - it just rereads SDR1 on every pteg lookup ;).
> >> There's no caching at all.
> > 
> > Ok, no idea why it's not working then.  I'll investigate when I get a chance.
> > 
> >> Of course, the guest needs to invalidate all pending tlb entries if they're
> >> now invalid.
> >> 
> >> Does this work on real hardware? Say, a G5?
> > 
> > As Paulus says it would be possible to do HPT resizing on real
> > hardware, but the implementation I've done is specific to PAPR.  And
> > obviously qemu wouldn't be relevant to that case.
> 
> So why make it specific to papr? Wouldn't it make sense to have it
> as a (ppc) generic interface in Linux?

Well, I sort of did, in that I added a ppc_md call for it.  I just
haven't implemented it for anything other than PAPR yet - the PAPR
implementation is quite different from what the native one would be,
since the hypervisor needs to handle the rehashing.

> For the PR PAPR case, QEMU allocates the HTAB, so it needs to make
> sure it pushes the changed address as new fake SDR1 value into kvm
> when it changes.

Yes, I'm doing that - have a look at the qemu series.  Not 100% sure
it's correct, since I haven't debugged with PR KVM yet.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]