On Tue, Oct 28, 2008 at 10:45:57AM -0500, Anthony Liguori wrote:
>> +ifeq ($(USE_KVM), 1)
>> +OBJS+= device-assignment.o
>> +endif
>
> I don't think you want to build this on PPC so I think you need a
> stronger check.
Good point. How about checking TARGET_BASE_ARCH = i386?
>> +static void assigned_dev_ioport_writel(void *opaque, uint32_t addr,
>> + uint32_t value)
>> +{
>> + AssignedDevRegion *r_access = opaque;
>> + uint32_t r_pio = guest_to_host_ioport(r_access, addr);
>> +
>> + DEBUG("%s: r_pio=%08x e_physbase=%08x r_virtbase=%08lx value=%08x\n",
>> + r_pio, (int)r_access->e_physbase,
>> + (unsigned long)r_access->r_virtbase, value);
>>
>
> The format doesn't match the parameter count.
Yep, already fixed.
>> +static void assigned_dev_ioport_map(PCIDevice *pci_dev, int region_num,
>> + uint32_t addr, uint32_t size, int
>> type)
>> +{
>> + AssignedDevice *r_dev = (AssignedDevice *) pci_dev;
>> + AssignedDevRegion *region = &r_dev->v_addrs[region_num];
>> + uint32_t old_port = region->u.r_baseport;
>> + uint32_t old_num = region->e_size;
>> + int first_map = (old_num == 0);
>> + struct ioperm_data data;
>> + int i;
>> +
>> + region->e_physbase = addr;
>> + region->e_size = size;
>> +
>> + DEBUG("e_phys=0x%x r_baseport=%x type=0x%x len=%d region_num=%d \n",
>> + addr, region->u.r_baseport, type, size, region_num);
>> +
>> + memset(&data, 0, sizeof(data));
>> +
>> + if (!first_map) {
>> + data.start_port = old_port;
>> + data.num = old_num; + data.turn_on = 0;
>> +
>> + for (i = 0; i < smp_cpus; ++i)
>> + kvm_ioperm(qemu_kvm_cpu_env(i), &data);
>>
>
> How does this interact with VCPU hot-plug?
I have no idea. Weidong?
>> +#ifdef KVM_CAP_IOMMU
>> + /* We always enable the IOMMU if present
>> + * (or when not disabled on the command line)
>> + */
>> + r = kvm_check_extension(kvm_context, KVM_CAP_IOMMU);
>> + if (r && !disable_iommu)
>> + assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
>> +#endif
>> + r = kvm_assign_pci_device(kvm_context, &assigned_dev_data);
>> + if (r < 0) {
>> + fprintf(stderr, "Could not notify kernel about "
>> + "assigned device \"%s\"\n", e_dev_name);
>> + perror("register_real_device");
>> + goto out;
>> + }
>> + }
>>
>
> You still succeed if KVM_CAP_DEVICE_ASSIGNMENT isn't defined? That
> means a newer userspace compiled on an older kernel will silently
> fail if they try to do device assignment. There's probably no
> reason to build this file if KVM_CAP_DEVICE_ASSIGNMENT isn't defined
> (see how the in-kernel PIT gets conditionally build depending on
> whether that cap is available).
Ok, I'll take a look at this.
>> +#endif
>> + term_printf("Registered host PCI device %02x:%02x.%1x "
>> + "(\"%s\") as guest device %02x:%02x.%1x\n",
>> + r_bus, r_dev, r_func, e_dev_name,
>> + pci_bus_num(e_bus), e_device, r_func);
>>
>>
>
> If I read the code correctly, this term_printf() happens regardless
> of whether this is being done for PCI hotplug or for command-line
> assignment? That's a problem as it'll print garbage on the monitor
> when you start QEMU which could break management applications.
Is there a more suitable alternative or shall I just nuke it?
>> diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c
>> index d559f0c..5fdb726 100644
>> --- a/qemu/hw/pc.c
>> +++ b/qemu/hw/pc.c
>> @@ -33,6 +33,7 @@
>> #include "boards.h"
>> #include "console.h"
>> #include "fw_cfg.h"
>> +#include "device-assignment.h"
>> #include "qemu-kvm.h"
>> @@ -1157,6 +1158,21 @@ static void pc_init1(ram_addr_t ram_size, int
>> vga_ram_size,
>> if (pci_enabled)
>> virtio_balloon_init(pci_bus);
>> +
>> + if (kvm_enabled() && device_assignment_enabled) {
>> + int i;
>>
>
> Stray tab.
Grrr. Silly emacs.
>
>> + for (i = 0; i < assigned_devices_index; i++) {
>> + if (add_assigned_device(assigned_devices[i]) < 0) {
>> + fprintf(stderr, "Warning: could not add assigned device
>> %s\n",
>> + assigned_devices[i]);
>> + }
>> + }
>> +
>> + if (init_all_assigned_devices(pci_bus)) {
>> + fprintf(stderr, "Failed to initialize assigned devices\n");
>> + exit (1);
>> + }
>> + }
>> }
>> +#if defined(TARGET_I386) || defined(TARGET_X86_64) || defined(__linux__)
>> + case QEMU_OPTION_pcidevice:
>> + device_assignment_enabled = 1;
>> + if (assigned_devices_index >= MAX_DEV_ASSIGN_CMDLINE) {
>> + fprintf(stderr, "Too many assigned devices\n");
>> + exit(1);
>> + }
>> + assigned_devices[assigned_devices_index] = optarg;
>> + assigned_devices_index++;
>> + break;
>>
>
> Tab damage.
Thanks, will fix in the next revision.
Cheers,
Muli
--
The First Workshop on I/O Virtualization (WIOV '08)
Dec 2008, San Diego, CA, http://www.usenix.org/wiov08/
<->
SYSTOR 2009---The Israeli Experimental Systems Conference
http://www.haifa.il.ibm.com/conferences/systor2009/
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
