[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC][PATCH 23/45] qemu-kvm: Rework MSI-X mask notifier
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [RFC][PATCH 23/45] qemu-kvm: Rework MSI-X mask notifier to generic MSI config notifiers |
Date: |
Tue, 18 Oct 2011 15:49:54 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2011-10-18 15:46, Michael S. Tsirkin wrote:
>>>> To
>>>> have common types, I would prefer to stay with vector config notifiers
>>>> as name then.
>>>>
>>>> Jan
>>>
>>> So we pass in nonsense values and ask all users to know about MSIX rules.
>>> Ugh.
>>>
>>> I do realize msi might change the vector without masking.
>>> We can either artificially call mask before value change
>>> and unmask after, or use 3 notifiers: mask,unmask,config.
>>> Add a comment that config is invoked when configuration
>>> for an unmasked vector is changed, and that
>>> it can only happen for msi, not msix.
>>
>> I see no need in complicating the API like this. MSI-X still needs the
>> config information on unmask, so let's just consistently pass it via the
>> unified config notifier instead of forcing the consumers to create yet
>> two more handlers. I really do not see the benefit for the consumer.
>>
>> Jan
>>
>
> The benefit is a clearer API, where all parameters you get are valid,
> so you do not need to go read the spec to see what is OK to use.
> Generally, encoding events in flags is more error
> prone than using different notifiers for different events.
>
> E.g. _unmask and _mask make
> it obvious that they are called on mask and on unmask
> respectively.
> OTOH _config_change(bool mask) is unclear: is 'mask' the new
> state or the old state?
>
> It might be just my taste, but I usually prefer multiple
> functions doing one thing each rather than a single
> function doing multiple things. It shouldn't be too hard ...
The impact on the user side (device models) will be larger while the
work in those different handlers will be widely the same (check my
series, e.g. the virtio handler). But it's still just a guess of mine as
well.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
- [Qemu-devel] [RFC][PATCH 31/45] qemu-kvm: Refactor kvm_deassign_irq to kvm_device_irq_deassign, (continued)
- [Qemu-devel] [RFC][PATCH 31/45] qemu-kvm: Refactor kvm_deassign_irq to kvm_device_irq_deassign, Jan Kiszka, 2011/10/17
- [Qemu-devel] [RFC][PATCH 29/45] pci-assign: Drop kvm_assigned_irq::host_irq initialization, Jan Kiszka, 2011/10/17
- [Qemu-devel] [RFC][PATCH 24/45] qemu-kvm: msix: Don't handle mask updated while disabled, Jan Kiszka, 2011/10/17
- [Qemu-devel] [RFC][PATCH 30/45] pci-assign: Rename assign_irq to assign_intx, Jan Kiszka, 2011/10/17
- [Qemu-devel] [RFC][PATCH 23/45] qemu-kvm: Rework MSI-X mask notifier to generic MSI config notifiers, Jan Kiszka, 2011/10/17
[Qemu-devel] [RFC][PATCH 41/45] msix: Drop unused msix_bar_size, Jan Kiszka, 2011/10/17
[Qemu-devel] [RFC][PATCH 33/45] qemu-kvm: Factor out kvm_device_intx_assign, Jan Kiszka, 2011/10/17
[Qemu-devel] [RFC][PATCH 05/45] msi: Invoke msi/msix_write_config from PCI core, Jan Kiszka, 2011/10/17
[Qemu-devel] [RFC][PATCH 27/45] qemu-kvm: Lazily update MSI caches, Jan Kiszka, 2011/10/17
[Qemu-devel] [RFC][PATCH 35/45] pci-assign: Polish assigned_dev_update_msix_mmio, Jan Kiszka, 2011/10/17
[Qemu-devel] [RFC][PATCH 40/45] qemu-kvm: msix: Drop check for preexisting cap from msix_add_config, Jan Kiszka, 2011/10/17
[Qemu-devel] [RFC][PATCH 18/45] qemu-kvm: Hook into MSI delivery at APIC level, Jan Kiszka, 2011/10/17
[Qemu-devel] [RFC][PATCH 32/45] pci-assign: Factor out deassign_irq, Jan Kiszka, 2011/10/17
[Qemu-devel] [RFC][PATCH 37/45] qemu-kvm: Clean up irqrouting API, Jan Kiszka, 2011/10/17
[Qemu-devel] [RFC][PATCH 39/45] pci-assign: Use generic MSI support, Jan Kiszka, 2011/10/17