Re: [RFC PATCH] Disable Book-E KVM support?

["Nicholas Piggin" <npiggin@gmail.com>]

On Thu Dec 1, 2022 at 6:45 AM AEST, Crystal Wood wrote:
> On Mon, 2022-11-28 at 14:36 +1000, Nicholas Piggin wrote:
> > BookE KVM is in a deep maintenance state, I'm not sure how much testing
> > it gets. I don't have a test setup, and it does not look like QEMU has
> > any HV architecture enabled. It hasn't been too painful but there are
> > some cases where it causes a bit of problem not being able to test, e.g.,
> > 
> > https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-November/251452.html
> > 
> > Time to begin removal process, or are there still people using it? I'm
> > happy to to keep making occasional patches to try keep it going if
> > there are people testing upstream. Getting HV support into QEMU would
> > help with long term support, not sure how big of a job that would be.
>
> Not sure what you mean about QEMU not having e500 HV support?  I don't know if
> it's bitrotted, but it's there.
>
> I don't know whether anyone is still using this, but if they are, it's
> probably e500mc and not e500v2 (which involved a bunch of hacks to get almost-
> sorta-usable performance out of hardware not designed for virtualization).  I
> do see that there have been a few recent patches on QEMU e500 (beyond the
> treewide cleanup type stuff), though I don't know if they're using KVM.  CCing
> them and the QEMU list.

Well I could be wrong about it, but it doesn't look it implements LPIDR
or GSPRs. The only use of MSR_GS seems to be a couple of places
including an instruction that aborts because no HV implementation. It
does have an MMU index selector but before d764184ddb22 that apparently
didn't really work.

QEMU probably should be able to run BookE KVM in PR mode at least.

> I have an e6500 I could occasionally test on, if it turns out people do still
> care about this.  Don't count me as the use case, though. :-)

Do you have a KVM setup on it? And it works with recentish upstream?

> FWIW, as far as the RECONCILE_IRQ_STATE issue, that used to be done in
> kvmppc_handle_exit(), but was moved in commit 9bd880a2c882 to be "cleaner and
> faster". :-P

Yeah that was probably reasonable at the time, that was the common way
to do it and thie patch avoids an unnecessary expensive write to MSR
(which my patch retains).

I think it must have always clobbered r4 though. It's possible it wasn't
tested with the right build option, or the right tracer active, or maybe
the call was simple enough that it was lucky and the compiler didn't use
r4. Easy bug to miss when it's not obvious that macro can call into C.

Thanks,
Nick