From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34395) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abSBS-00053x-L8 for qemu-devel@nongnu.org; Thu, 03 Mar 2016 07:16:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abSBP-0007Bp-Fd for qemu-devel@nongnu.org; Thu, 03 Mar 2016 07:16:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54165) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abSBP-0007BD-7p for qemu-devel@nongnu.org; Thu, 03 Mar 2016 07:16:43 -0500 References: <1456078260-6669-1-git-send-email-davidkiarie4@gmail.com> <1456078260-6669-2-git-send-email-davidkiarie4@gmail.com> <56CF212B.2000400@redhat.com> <56D73ACA.8030109@gmail.com> From: Marcel Apfelbaum Message-ID: <56D82B27.9070302@redhat.com> Date: Thu, 3 Mar 2016 14:16:39 +0200 MIME-Version: 1.0 In-Reply-To: <56D73ACA.8030109@gmail.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [V6 1/4] hw/i386: Introduce AMD IOMMU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Kiarie , qemu-devel@nongnu.org Cc: valentine.sinitsyn@gmail.com, jan.kiszka@web.de, mst@redhat.com On 03/02/2016 09:11 PM, David Kiarie wrote: > > > On 25/02/16 18:43, Marcel Apfelbaum wrote: >> On 02/21/2016 08:10 PM, David Kiarie wrote: >>> Add AMD IOMMU emulaton to Qemu in addition to Intel IOMMU >>> The IOMMU does basic translation, error checking and has a >>> mininal IOTLB implementation >> >> Hi, >> >>> >>> Signed-off-by: David Kiarie >>> --- > [...] >>> + >>> +static void amd_iommu_realize(PCIDevice *dev, Error **error) >>> +{ >>> + AMDIOMMUState *s = container_of(dev, AMDIOMMUState, dev); >>> + >>> + s->iotlb = g_hash_table_new_full(amd_iommu_uint64_hash, >>> + amd_iommu_uint64_equal, g_free, g_free); >>> + >>> + s->capab_offset = pci_add_capability(dev, IOMMU_CAPAB_ID_SEC, 0, >>> + IOMMU_CAPAB_SIZE); >>> + >>> + /* add msi and hypertransport capabilities */ >>> + pci_add_capability(&s->dev, PCI_CAP_ID_MSI, 0, IOMMU_CAPAB_REG_SIZE); >>> + pci_add_capability(&s->dev, PCI_CAP_ID_HT, 0, IOMMU_CAPAB_REG_SIZE); >>> + >>> + amd_iommu_init(s); >>> + >>> + /* set up MMIO */ >>> + memory_region_init_io(&s->mmio, OBJECT(s), &mmio_mem_ops, s, "mmio", >>> + IOMMU_MMIO_SIZE); >>> + >>> + if (s->mmio.addr == IOMMU_BASE_ADDR) { >> >> I don't understand why is need here. realize is called only once in the init process >> and you set it a few lines below. >> >>> + return; >>> + } >>> + >>> + s->mmio.addr = IOMMU_BASE_ADDR; >>> + memory_region_add_subregion(get_system_memory(), IOMMU_BASE_ADDR, &s->mmio); >>> +} >>> + >>> +static const VMStateDescription vmstate_amd_iommu = { >>> + .name = "amd-iommu", >>> + .fields = (VMStateField[]) { >>> + VMSTATE_PCI_DEVICE(dev, AMDIOMMUState), >>> + VMSTATE_END_OF_LIST() >>> + } >>> +}; >>> + >>> +static Property amd_iommu_properties[] = { >>> + DEFINE_PROP_UINT32("version", AMDIOMMUState, version, 2), >>> + DEFINE_PROP_END_OF_LIST(), >>> +}; >>> + >>> +static void amd_iommu_uninit(PCIDevice *dev) >>> +{ >>> + AMDIOMMUState *s = container_of(dev, AMDIOMMUState, dev); >>> + amd_iommu_iotlb_reset(s); >> >> at this point you also need to clean also the memory regions you use. > > What exactly do you mean by clean up the memory regions ? > You have an memory_region_add_subregion on realize, you need a memory_region_del_subregion on exit. Thanks, Marcel >> >>> +} >>> + >>> +static void amd_iommu_class_init(ObjectClass *klass, void* data) >>> +{ >>> + DeviceClass *dc = DEVICE_CLASS(klass); >>> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >>> + >>> + k->realize = amd_iommu_realize; >>> + k->exit = amd_iommu_uninit; >>> + >>> + dc->reset = amd_iommu_reset; >>> + dc->vmsd = &vmstate_amd_iommu; >>> + dc->props = amd_iommu_properties; >>> +} >>> + >>> +static const TypeInfo amd_iommu = { >>> + .name = TYPE_AMD_IOMMU_DEVICE, >>> + .parent = TYPE_PCI_DEVICE, >>> + .instance_size = sizeof(AMDIOMMUState), >>> + .class_init = amd_iommu_class_init >>> +}; >>> + >>> +static void amd_iommu_register_types(void) >>> +{ >>> + type_register_static(&amd_iommu); >>> +} >>> + >>> +type_init(amd_iommu_register_types); [...]