From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57290) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y9Z4J-0001Vc-Et for qemu-devel@nongnu.org; Fri, 09 Jan 2015 07:53:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y9Z4F-00074Y-5J for qemu-devel@nongnu.org; Fri, 09 Jan 2015 07:53:35 -0500 Received: from mail-pa0-f47.google.com ([209.85.220.47]:43270) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y9Z4E-000703-U7 for qemu-devel@nongnu.org; Fri, 09 Jan 2015 07:53:31 -0500 Received: by mail-pa0-f47.google.com with SMTP id kq14so18364598pab.6 for ; Fri, 09 Jan 2015 04:53:29 -0800 (PST) Message-ID: <54AFCF42.1060206@ozlabs.ru> Date: Fri, 09 Jan 2015 23:53:22 +1100 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1420765371-25342-1-git-send-email-aik@ozlabs.ru> <54AFA7DD.1020803@redhat.com> In-Reply-To: <54AFA7DD.1020803@redhat.com> Content-Type: text/plain; charset=koi8-r Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH] spapr_vio/spapr_iommu: Move VIO bypass where it belongs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: qemu-ppc@nongnu.org, Alexander Graf , David Gibson On 01/09/2015 09:05 PM, Paolo Bonzini wrote: > > > On 09/01/2015 02:02, Alexey Kardashevskiy wrote: >> @@ -100,7 +98,7 @@ static const VMStateDescription vmstate_spapr_tce_table = { >> VMSTATE_UINT32_EQUAL(nb_table, sPAPRTCETable), >> >> /* IOMMU state */ >> - VMSTATE_BOOL(bypass, sPAPRTCETable), >> + VMSTATE_UNUSED(sizeof(bool)), > > sizeof(bool) can change across hosts, this needs to be the size that is > written by put_bool in migration/vmstate.c, i.e. VMSTATE_UNUSED(1). > >> VMSTATE_VARRAY_UINT32(table, sPAPRTCETable, nb_table, 0, vmstate_info_uint64, uint64_t), >> >> VMSTATE_END_OF_LIST() >> @@ -131,7 +129,8 @@ static int spapr_tce_table_realize(DeviceState *dev) >> trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd); >> >> memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops, >> - "iommu-spapr", ram_size); >> + "iommu-spapr", >> + (uint64_t)tcet->nb_table << tcet->page_shift); >> >> QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list); >> >> @@ -191,17 +190,11 @@ MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet) >> return &tcet->iommu; >> } >> >> -void spapr_tce_set_bypass(sPAPRTCETable *tcet, bool bypass) >> -{ >> - tcet->bypass = bypass; >> -} >> - >> static void spapr_tce_reset(DeviceState *dev) >> { >> sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev); >> size_t table_size = tcet->nb_table * sizeof(uint64_t); >> >> - tcet->bypass = false; >> memset(tcet->table, 0, table_size); >> } >> >> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c >> index dc9e46a..a1f2316 100644 >> --- a/hw/ppc/spapr_vio.c >> +++ b/hw/ppc/spapr_vio.c >> @@ -322,6 +322,18 @@ static void spapr_vio_quiesce_one(VIOsPAPRDevice *dev) >> free_crq(dev); >> } >> >> +static void spapr_vio_set_bypass(VIOsPAPRDevice *dev, bool bypass) >> +{ >> + if (dev->bypass == bypass) { >> + return; >> + } >> + >> + memory_region_set_enabled(&dev->mrbypass, bypass); >> + memory_region_set_enabled(spapr_tce_get_iommu(dev->tcet), !bypass); >> + >> + dev->bypass = bypass; >> +} >> + >> static void rtas_set_tce_bypass(PowerPCCPU *cpu, sPAPREnvironment *spapr, >> uint32_t token, >> uint32_t nargs, target_ulong args, >> @@ -348,7 +360,7 @@ static void rtas_set_tce_bypass(PowerPCCPU *cpu, sPAPREnvironment *spapr, >> return; >> } >> >> - spapr_tce_set_bypass(dev->tcet, !!enable); >> + spapr_vio_set_bypass(dev, !!enable); >> >> rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> } >> @@ -407,6 +419,7 @@ static void spapr_vio_busdev_reset(DeviceState *qdev) >> >> dev->signal_state = 0; >> >> + spapr_vio_set_bypass(dev, false); >> if (pc->reset) { >> pc->reset(dev); >> } >> @@ -456,14 +469,25 @@ static int spapr_vio_busdev_init(DeviceState *qdev) >> >> if (pc->rtce_window_size) { >> uint32_t liobn = SPAPR_VIO_BASE_LIOBN | dev->reg; >> + >> + memory_region_init(&dev->mrroot, OBJECT(dev), "iommu-spapr-root", >> + ram_size); >> + memory_region_init_alias(&dev->mrbypass, OBJECT(dev), >> + "iommu-spapr-bypass", get_system_memory(), >> + 0, ram_size); >> + memory_region_add_subregion_overlap(&dev->mrroot, 0, &dev->mrbypass, 1); >> + address_space_init(&dev->as, &dev->mrroot, qdev->id); >> + >> dev->tcet = spapr_tce_new_table(qdev, liobn, >> 0, >> SPAPR_TCE_PAGE_SHIFT, >> pc->rtce_window_size >> >> SPAPR_TCE_PAGE_SHIFT, false); >> - address_space_init(&dev->as, spapr_tce_get_iommu(dev->tcet), qdev->id); >> + memory_region_add_subregion_overlap(&dev->mrroot, 0, >> + spapr_tce_get_iommu(dev->tcet), 2); >> } >> >> + >> return pc->init(dev); >> } >> >> @@ -541,6 +565,15 @@ static void spapr_vio_bridge_class_init(ObjectClass *klass, void *data) >> k->init = spapr_vio_bridge_init; >> } >> >> +static int spapr_vio_post_load(void *opaque, int version_id) >> +{ >> + VIOsPAPRDevice *dev = VIO_SPAPR_DEVICE(opaque); >> + >> + spapr_vio_set_bypass(dev, dev->bypass); >> + >> + return 0; >> +} >> + >> static const TypeInfo spapr_vio_bridge_info = { >> .name = "spapr-vio-bridge", >> .parent = TYPE_SYS_BUS_DEVICE, >> @@ -550,8 +583,9 @@ static const TypeInfo spapr_vio_bridge_info = { >> >> const VMStateDescription vmstate_spapr_vio = { >> .name = "spapr_vio", >> - .version_id = 1, >> + .version_id = 2, >> .minimum_version_id = 1, > > I think the minimum version should be 2 as well, because migrating from > version 1 will not set the bypass field correctly. This why the patch is "RFC" :) I can keep the flag in TCETable which would be a bit ugly but won't break migration. Is there any better way to keep compatibility? > > Paolo > >> + .post_load = spapr_vio_post_load, >> .fields = (VMStateField[]) { >> /* Sanity check */ >> VMSTATE_UINT32_EQUAL(reg, VIOsPAPRDevice), >> @@ -563,6 +597,8 @@ const VMStateDescription vmstate_spapr_vio = { >> VMSTATE_UINT32(crq.qsize, VIOsPAPRDevice), >> VMSTATE_UINT32(crq.qnext, VIOsPAPRDevice), >> >> + VMSTATE_BOOL_V(bypass, VIOsPAPRDevice, 2), >> + >> VMSTATE_END_OF_LIST() >> }, -- Alexey