On Wed, Nov 28, 2018 at 11:45:46PM +0100, Cédric Le Goater wrote: > On 11/28/18 6:52 AM, David Gibson wrote: > > On Fri, Nov 16, 2018 at 11:57:15AM +0100, Cédric Le Goater wrote: > >> This introduces a set of XIVE models specific to KVM which derive from > >> the XIVE base models. The interfaces with KVM are a new capability and > >> a new KVM device for the XIVE native exploitation interrupt mode. > >> > >> They handle the initialization of the TIMA and the source ESB memory > >> regions which have a different type under KVM. These are 'ram device' > >> memory mappings, similarly to VFIO, exposed to the guest and the > >> associated VMAs on the host are populated dynamically with the > >> appropriate pages using a fault handler. > >> > >> Signed-off-by: Cédric Le Goater <c...@kaod.org> > > > > The logic here looks fine, but I think it would be better to activate > > it with explicit if (kvm) type logic rather than using a subclass. > > ok. ARM has taken a different path, the one proposed below, but it should > be possible to use a "if (kvm)" type logic. There should be less noise > in the object design.
Yeah, it seemed like a good path when I wrote the XICS code, but
experience with that has led me to decide it wasn't a good idea. I'm
not sure if the ARM people copied that or came up with it on their
own.
[snip]
> >> +/*
> >> + * XIVE Thread Interrupt Management context (KVM)
> >> + */
> >> +
> >> +static void xive_tctx_kvm_init(XiveTCTX *tctx, Error **errp)
> >> +{
> >> + sPAPRXive *xive;
> >> + unsigned long vcpu_id;
> >> + int ret;
> >> +
> >> + /* Check if CPU was hot unplugged and replugged. */
> >> + if (kvm_cpu_is_enabled(tctx->cs)) {
> >> + return;
> >> + }
> >> +
> >> + vcpu_id = kvm_arch_vcpu_id(tctx->cs);
> >> + xive = SPAPR_XIVE_KVM(tctx->xrtr);
> >
> > Is this the first use of tctx->xrtr?
>
> No, the second. the first is the reset_tctx() ops doing the CAM reset.
> But we said that we could remove it.
And I think we can remove it here too. We know this is PAPR specific
so we can go qdev_get_machine() -> PAPR xive object.
I normally don't like using qdev_get_machine(), but I think it's a
little less ugly than including this backlink (and it is sometimes
necessary to deal with the different abstraction boundaries between
qemu and KVM).
--
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
