On Thu, 8 Jun 2017 11:25:52 +0200 Cédric Le Goater <c...@kaod.org> wrote:
> On 06/08/2017 11:14 AM, Greg Kurz wrote: > > On Thu, 8 Jun 2017 11:53:29 +1000 > > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > >> On Wed, Jun 07, 2017 at 10:55:07PM +0200, Greg Kurz wrote: > >>> On Wed, 7 Jun 2017 20:11:38 +0200 > >>> Cédric Le Goater <c...@kaod.org> wrote: > >>> > >>>> On 06/07/2017 07:17 PM, Greg Kurz wrote: > >>>>> Until recently, spapr used to allocate ICPState objects for the lifetime > >>>>> of the machine. They would only be associated to vCPUs in > >>>>> xics_cpu_setup() > >>>>> when plugging a CPU core. > >>>>> > >>>>> Now that ICPState objects have the same lifecycle as vCPUs, it is > >>>>> possible to associate them during realization. > >>>>> > >>>>> This patch hence open-codes xics_cpu_setup() in icp_realize(). The vCPU > >>>>> is passed as a property. Note that vCPU now needs to be realized first > >>>>> for the IRQs to be allocated. It also needs to resetted before ICPState > >>>>> realization in order to synchronize with KVM. > >>>>> > >>>>> Since ICPState objects are freed when unrealized, xics_cpu_destroy() > >>>>> isn't > >>>>> needed anymore and can be safely dropped. > >>>> > >>>> I like the idea but I think the assignment of ->cs attribute should be > >>>> moved under icp_kvm_cpu_setup(), as it is only used under xics_kvm by > >>>> the kvm_vcpu_ioctl() calls. > >>>> > >>> > >>> Well, cs->cpu_index is also used for traces and to implement the 'info > >>> pic' > >>> monitor command. > >> > >> Right. I think it makes sense for the ICP to have a handle on it's > >> associated CPU, even if we don't actually use it in all cases right > >> now. So I have no problem with the property being in all ICPs; I > >> think that will be cleaner than special casing xics_kvm. Especially > >> if we have to un-special-case it sometime in future because we need > >> to access the CPU object for some reason we haven't thought of yet. > >> > > > > We had discussed with Cedric about that actually but when I started > > to write code, I had the impression that I would have to do convoluted > > things to get rid of the CPU handle in ICP. > > There is nothing complex and it would surely simplify cpu_setup(). > > But, the main argument is that it might be useful to other platforms. > So there is not much value in removing it. I am OK with that. I am less > OK with using the index, even if it is useful for the migration state > of ICP objects today. > > We could be tempted to use more of cpu_index, like we have done in > the past, and that was really complex to untangle. Let's be cautious. > I agree. Maybe this could be hidden in a icpc->get_index() handler ? > C.
pgpfSRj_FbzuM.pgp
Description: OpenPGP digital signature
