On Thu, Aug 14, 2014 at 07:33:29PM +0800, Le Tan wrote: > 2014-08-14 19:12 GMT+08:00 Michael S. Tsirkin <m...@redhat.com>: > > On Mon, Aug 11, 2014 at 03:05:01PM +0800, Le Tan wrote: > >> Add Intel IOMMU emulation to q35 chipset and expose it to the guest. > >> 1. Add a machine option. Users can use "-machine iommu=on|off" in the > >> command > >> line to enable/disable Intel IOMMU. The default is off. > >> 2. Accroding to the machine option, q35 will initialize the Intel IOMMU and > >> use pci_setup_iommu() to setup q35_host_dma_iommu() as the IOMMU function > >> for > >> the pci bus. > >> 3. q35_host_dma_iommu() will return different address space according to > >> the > >> bus_num and devfn of the device. > >> > >> Signed-off-by: Le Tan <tamlokv...@gmail.com> > >> --- > >> hw/core/machine.c | 27 +++++++++++++++++--- > >> hw/pci-host/q35.c | 64 > >> +++++++++++++++++++++++++++++++++++++++++++---- > >> include/hw/boards.h | 1 + > >> include/hw/pci-host/q35.h | 2 ++ > >> qemu-options.hx | 5 +++- > >> vl.c | 4 +++ > >> 6 files changed, 94 insertions(+), 9 deletions(-) > >> > >> diff --git a/hw/core/machine.c b/hw/core/machine.c > >> index 7a66c57..f0046d6 100644 > >> --- a/hw/core/machine.c > >> +++ b/hw/core/machine.c > >> @@ -235,6 +235,20 @@ static void machine_set_firmware(Object *obj, const > >> char *value, Error **errp) > >> ms->firmware = g_strdup(value); > >> } > >> > >> +static bool machine_get_iommu(Object *obj, Error **errp) > >> +{ > >> + MachineState *ms = MACHINE(obj); > >> + > >> + return ms->iommu; > >> +} > >> + > >> +static void machine_set_iommu(Object *obj, bool value, Error **errp) > >> +{ > >> + MachineState *ms = MACHINE(obj); > >> + > >> + ms->iommu = value; > >> +} > >> + > >> static void machine_initfn(Object *obj) > >> { > >> object_property_add_str(obj, "accel", > >> @@ -270,10 +284,17 @@ static void machine_initfn(Object *obj) > >> machine_set_dump_guest_core, > >> NULL); > >> object_property_add_bool(obj, "mem-merge", > >> - machine_get_mem_merge, > >> machine_set_mem_merge, NULL); > >> - object_property_add_bool(obj, "usb", machine_get_usb, > >> machine_set_usb, NULL); > >> + machine_get_mem_merge, > >> + machine_set_mem_merge, NULL); > >> + object_property_add_bool(obj, "usb", > >> + machine_get_usb, > >> + machine_set_usb, NULL); > >> object_property_add_str(obj, "firmware", > >> - machine_get_firmware, machine_set_firmware, > >> NULL); > >> + machine_get_firmware, > >> + machine_set_firmware, NULL); > >> + object_property_add_bool(obj, "iommu", > >> + machine_get_iommu, > >> + machine_set_iommu, NULL); > >> } > >> > >> static void machine_finalize(Object *obj) > >> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > >> index a0a3068..3342711 100644 > >> --- a/hw/pci-host/q35.c > >> +++ b/hw/pci-host/q35.c > >> @@ -347,6 +347,53 @@ static void mch_reset(DeviceState *qdev) > >> mch_update(mch); > >> } > >> > >> +static AddressSpace *q35_host_dma_iommu(PCIBus *bus, void *opaque, int > >> devfn) > >> +{ > >> + IntelIOMMUState *s = opaque; > >> + VTDAddressSpace **pvtd_as; > >> + VTDAddressSpace *vtd_as; > >> + int bus_num = pci_bus_num(bus); > >> + > >> + assert(devfn >= 0); > >> + > >> + pvtd_as = s->address_spaces[bus_num]; > >> + if (!pvtd_as) { > >> + /* No corresponding free() */ > >> + pvtd_as = g_malloc0(sizeof(VTDAddressSpace *) * > >> + VTD_PCI_SLOT_MAX * VTD_PCI_FUNC_MAX); > >> + s->address_spaces[bus_num] = pvtd_as; > >> + } > >> + > >> + vtd_as = *(pvtd_as + devfn); > > > > pvtd_as[devfn] is cleaner. > > In fact, you can then drop vtd_as local variable, use pvtd_as[devfn] > > directly. > > > >> + if (!vtd_as) { > >> + vtd_as = g_malloc0(sizeof(*vtd_as)); > >> + *(pvtd_as + devfn) = vtd_as; > >> + > >> + vtd_as->bus_num = bus_num; > >> + vtd_as->devfn = devfn; > >> + vtd_as->iommu_state = s; > >> + memory_region_init_iommu(&vtd_as->iommu, OBJECT(s), &s->iommu_ops, > >> + "intel_iommu", UINT64_MAX); > >> + address_space_init(&vtd_as->as, &vtd_as->iommu, "intel_iommu"); > >> + } > >> + > >> + return &vtd_as->as; > >> +} > >> + > >> +static void mch_init_dmar(MCHPCIState *mch) > >> +{ > >> + PCIBus *pci_bus = PCI_BUS(qdev_get_parent_bus(DEVICE(mch))); > >> + > >> + mch->iommu = INTEL_IOMMU_DEVICE(qdev_create(NULL, > >> TYPE_INTEL_IOMMU_DEVICE)); > >> + object_property_add_child(OBJECT(mch), "intel-iommu", > >> + OBJECT(mch->iommu), NULL); > >> + qdev_init_nofail(DEVICE(mch->iommu)); > >> + sysbus_mmio_map(SYS_BUS_DEVICE(mch->iommu), 0, > >> Q35_HOST_BRIDGE_IOMMU_ADDR); > >> + > >> + pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch->iommu); > >> +} > >> + > >> + > >> static int mch_init(PCIDevice *d) > >> { > >> int i; > >> @@ -363,13 +410,20 @@ static int mch_init(PCIDevice *d) > >> memory_region_add_subregion_overlap(mch->system_memory, 0xa0000, > >> &mch->smram_region, 1); > >> memory_region_set_enabled(&mch->smram_region, false); > >> - init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory, > >> mch->pci_address_space, > >> - &mch->pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE); > >> + init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory, > >> + mch->pci_address_space, &mch->pam_regions[0], PAM_BIOS_BASE, > >> + PAM_BIOS_SIZE); > >> for (i = 0; i < 12; ++i) { > >> - init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory, > >> mch->pci_address_space, > >> - &mch->pam_regions[i+1], PAM_EXPAN_BASE + i * > >> PAM_EXPAN_SIZE, > >> - PAM_EXPAN_SIZE); > >> + init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory, > >> + mch->pci_address_space, &mch->pam_regions[i+1], > >> + PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE); > > > > Random formatting changes above? Make it a separate patch please. > > Yes, I was told that I could fix some coding style issues around > places that my patch touches. If that is improper, I will just first > leave them out. :) > Thanks for your reviews! > > Le
Review's easier if they are split out in a separate patch. No need to discard this, just split. > >> + } > >> + > >> + /* Intel IOMMU (VT-d) */ > >> + if (qemu_opt_get_bool(qemu_get_machine_opts(), "iommu", false)) { > >> + mch_init_dmar(mch); > >> } > >> + > >> return 0; > >> } > >> > >> diff --git a/include/hw/boards.h b/include/hw/boards.h > >> index 605a970..dfb6718 100644 > >> --- a/include/hw/boards.h > >> +++ b/include/hw/boards.h > >> @@ -123,6 +123,7 @@ struct MachineState { > >> bool mem_merge; > >> bool usb; > >> char *firmware; > >> + bool iommu; > >> > >> ram_addr_t ram_size; > >> ram_addr_t maxram_size; > >> diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h > >> index d9ee978..025d6e6 100644 > >> --- a/include/hw/pci-host/q35.h > >> +++ b/include/hw/pci-host/q35.h > >> @@ -33,6 +33,7 @@ > >> #include "hw/acpi/acpi.h" > >> #include "hw/acpi/ich9.h" > >> #include "hw/pci-host/pam.h" > >> +#include "hw/i386/intel_iommu.h" > >> > >> #define TYPE_Q35_HOST_DEVICE "q35-pcihost" > >> #define Q35_HOST_DEVICE(obj) \ > >> @@ -60,6 +61,7 @@ typedef struct MCHPCIState { > >> uint64_t pci_hole64_size; > >> PcGuestInfo *guest_info; > >> uint32_t short_root_bus; > >> + IntelIOMMUState *iommu; > >> } MCHPCIState; > >> > >> typedef struct Q35PCIHost { > >> diff --git a/qemu-options.hx b/qemu-options.hx > >> index 96516c1..7406a17 100644 > >> --- a/qemu-options.hx > >> +++ b/qemu-options.hx > >> @@ -35,7 +35,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > >> " kernel_irqchip=on|off controls accelerated irqchip > >> support\n" > >> " kvm_shadow_mem=size of KVM shadow MMU\n" > >> " dump-guest-core=on|off include guest memory in a > >> core dump (default=on)\n" > >> - " mem-merge=on|off controls memory merge support > >> (default: on)\n", > >> + " mem-merge=on|off controls memory merge support > >> (default: on)\n" > >> + " iommu=on|off controls emulated Intel IOMMU (VT-d) > >> support (default=off)\n", > >> QEMU_ARCH_ALL) > >> STEXI > >> @item -machine [type=]@var{name}[,prop=@var{value}[,...]] > >> @@ -58,6 +59,8 @@ Include guest memory in a core dump. The default is on. > >> Enables or disables memory merge support. This feature, when supported by > >> the host, de-duplicates identical memory pages among VMs instances > >> (enabled by default). > >> +@item iommu=on|off > >> +Enables or disables emulated Intel IOMMU (VT-d) support. The default is > >> off. > >> @end table > >> ETEXI > >> > >> diff --git a/vl.c b/vl.c > >> index a8029d5..2ab1643 100644 > >> --- a/vl.c > >> +++ b/vl.c > >> @@ -388,6 +388,10 @@ static QemuOptsList qemu_machine_opts = { > >> .name = PC_MACHINE_MAX_RAM_BELOW_4G, > >> .type = QEMU_OPT_SIZE, > >> .help = "maximum ram below the 4G boundary (32bit boundary)", > >> + },{ > >> + .name = "iommu", > >> + .type = QEMU_OPT_BOOL, > >> + .help = "Set on/off to enable/disable Intel IOMMU (VT-d)", > >> }, > >> { /* End of list */ } > >> }, > >> -- > >> 1.9.1