2014-08-14 19:35 GMT+08:00 Michael S. Tsirkin <m...@redhat.com>:
> 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.
OK, get it.
Le
>> >> + }
>> >> +
>> >> + /* 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
