Hi Sheng,
On Tue, Dec 23, 2008 at 04:00:25PM +0800, Sheng Yang wrote:
> Which is more convenient...
>
> Signed-off-by: Sheng Yang <[email protected]>
> ---
> virt/kvm/kvm_main.c | 10 ++--------
> 1 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index ffd261d..cd84b3e 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -284,11 +284,7 @@ static int assigned_device_update_intx(struct kvm *kvm,
> return 0;
>
> if (irqchip_in_kernel(kvm)) {
> - if (!msi2intx &&
> - adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI) {
> - free_irq(adev->host_irq, (void *)kvm);
> - pci_disable_msi(adev->dev);
> - }
> + kvm_free_assigned_irq(kvm, adev);
>
> if (!capable(CAP_SYS_RAWIO))
> return -EPERM;
> @@ -339,9 +335,7 @@ static int assigned_device_update_msi(struct kvm *kvm,
>
> if (irqchip_in_kernel(kvm)) {
> if (!msi2intx) {
> - if (adev->irq_requested_type &
> - KVM_ASSIGNED_DEV_HOST_INTX)
> - free_irq(adev->host_irq, (void *)adev);
> + kvm_free_assigned_irq(kvm, adev);
>
> r = pci_enable_msi(adev->dev);
> if (r)
Regarding kvm_free_assigned_irq and
assigned_device_update_msi/update_intx:
if (cancel_work_sync(&assigned_dev->interrupt_work))
/* We had pending work. That means we will have to take
* care of kvm_put_kvm.
*/
kvm_put_kvm(kvm);
free_irq(assigned_dev->host_irq, (void *)assigned_dev);
What prevents the host IRQ from being triggered between kvm_put_kvm and
free_irq?
Also, if the kvm_put_kvm(kvm) from
kvm_assigned_dev_interrupt_work_handler happens to be the last one,
can't this happen:
- kvm_assigned_dev_interrupt_work_handler
- kvm_put_kvm
- kvm_destroy_vm
- kvm_arch_destroy_vm
- kvm_free_all_assigned_devices
- kvm_free_assigned_device
- kvm_free_assigned_irq
- cancel_work_sync(&assigned_dev->interrupt_work)
deadlock.
The msi2intx parameter is somewhat odd. Wouldnt it be more versatile
if controlled from userspace (and per guest) ?
--
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
