On Wed, 2008-12-10 at 17:45 +0800, Han, Weidong wrote:
> assign_dev_update_irq may not be invoked when hot add a device, so
> need to assign irq after assign device in init_assigned_device.
Makes sense, but ...
> Signed-off-by: Weidong Han <[EMAIL PROTECTED]>
> ---
> qemu/hw/device-assignment.c | 99 ++++++++++++++++++++++++++++--------------
> 1 files changed, 66 insertions(+), 33 deletions(-)
>
> diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
> index 20618a4..557beb8 100644
> --- a/qemu/hw/device-assignment.c
> +++ b/qemu/hw/device-assignment.c
> @@ -490,6 +490,65 @@ static uint32_t calc_assigned_dev_id(uint8_t bus,
> uint8_t devfn)
> return (uint32_t)bus << 8 | (uint32_t)devfn;
> }
>
...
> +static int assign_irq(AssignedDevInfo *adev)
> +{
> + struct kvm_assigned_irq assigned_irq_data;
> + AssignedDevice *dev = adev->assigned_dev;
> + int irq, r = 0;
> +
> + irq = pci_map_irq(&dev->dev, dev->intpin);
> + irq = piix_get_irq(irq);
> +
> +#ifdef TARGET_IA64
> + irq = ipf_map_irq(&dev->dev, irq);
> +#endif
If you added here:
if (irq == dev->girq)
return;
then ...
> + memset(&assigned_irq_data, 0, sizeof(assigned_irq_data));
> + assigned_irq_data.assigned_dev_id =
> + calc_assigned_dev_id(dev->h_busnr, dev->h_devfn);
> + assigned_irq_data.guest_irq = irq;
> + assigned_irq_data.host_irq = dev->real_device.irq;
> + r = kvm_assign_irq(kvm_context, &assigned_irq_data);
> + if (r < 0) {
> + fprintf(stderr, "Failed to assign irq for \"%s\": %s\n",
> + adev->name, strerror(-r));
> + fprintf(stderr, "Perhaps you are assigning a device "
> + "that shares an IRQ with another device?\n");
> + return r;
> + }
> +
> + dev->girq = irq;
> + return r;
> +}
> +
> /* The pci config space got updated. Check if irq numbers have changed
> * for our devices
> */
> @@ -511,20 +570,8 @@ void assigned_dev_update_irq()
> #endif
>
> if (irq != assigned_dev->girq) {
> - struct kvm_assigned_irq assigned_irq_data;
> -
> - memset(&assigned_irq_data, 0, sizeof(assigned_irq_data));
> - assigned_irq_data.assigned_dev_id =
> - calc_assigned_dev_id(assigned_dev->h_busnr,
> - (uint8_t) assigned_dev->h_devfn);
> - assigned_irq_data.guest_irq = irq;
> - assigned_irq_data.host_irq = assigned_dev->real_device.irq;
> - r = kvm_assign_irq(kvm_context, &assigned_irq_data);
> + r = assign_irq(adev);
> if (r < 0) {
> - fprintf(stderr, "Failed to assign irq for \"%s\": %s\n",
> - adev->name, strerror(-r));
> - fprintf(stderr, "Perhaps you are assigning a device "
> - "that shares an IRQ with another device?\n");
> free_assigned_device(adev);
> adev = next;
> continue;
.... you end up with:
void assigned_dev_update_irq(PCIDevice *d)
{
AssignedDevInfo *adev;
adev = LIST_FIRST(&adev_head);
while (adev) {
AssignedDevInfo *next = LIST_NEXT(adev, next);
int r;
r = assign_irq(adev);
if (r < 0)
free_assigned_device(adev);
adev = next;
}
which is much nicer.
> @@ -579,27 +626,13 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo
> *adev, PCIBus *bus)
> dev->h_busnr = adev->bus;
> dev->h_devfn = PCI_DEVFN(adev->dev, adev->func);
>
> - memset(&assigned_dev_data, 0, sizeof(assigned_dev_data));
> - assigned_dev_data.assigned_dev_id =
> - calc_assigned_dev_id(dev->h_busnr, (uint32_t)dev->h_devfn);
> - assigned_dev_data.busnr = dev->h_busnr;
> - assigned_dev_data.devfn = dev->h_devfn;
> -
> -#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 && !adev->disable_iommu)
> - assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
> -#endif
> + r = assign_device(adev);
> + if (r < 0)
> + return NULL;
>
> - r = kvm_assign_pci_device(kvm_context, &assigned_dev_data);
> - if (r < 0) {
> - fprintf(stderr, "Failed to assign device \"%s\" : %s\n",
> - adev->name, strerror(-r));
> - return NULL;
> - }
> + r = assign_irq(adev);
> + if (r < 0)
> + return NULL;
Hmm, why are we doing no cleanup if these fail?
Cheers,
Mark.
--
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
