On Thu, Nov 22, 2012 at 7:40 PM, Jan Kiszka <jan.kis...@siemens.com> wrote:
> On 2012-11-22 03:48, liu ping fan wrote:
>> On Wed, Nov 21, 2012 at 8:43 PM, Jan Kiszka <jan.kis...@siemens.com> wrote:
>>> On 2012-11-21 07:02, Liu Ping Fan wrote:
[...]
>>>>
>>>> +static int ivshmem_vector_use(PCIDevice *dev, unsigned vector,
>>>> + MSIMessage msg)
>>>> +{
>>>> + IVShmemState *s = DO_UPCAST(IVShmemState, dev, dev);
>>>> + int virq;
>>>> + EventNotifier *n = &s->peers[s->vm_id].eventfds[vector];
>>>> +
>>>> + virq = kvm_irqchip_add_msi_route(kvm_state, msg);
>>>> + if (virq >= 0 && kvm_irqchip_add_irqfd_notifier(kvm_state, n, virq)
>>>> >= 0) {
>>>> + s->vector_irqfd[vector].virq = virq;
>>>> + s->vector_irqfd[vector].used = true;
>>>> + qemu_chr_add_handlers(s->eventfd_chr[vector], NULL, NULL, NULL,
>>>> NULL);
>>>> + } else if (virq >= 0) {
>>>> + kvm_irqchip_release_virq(kvm_state, virq);
>>>> + }
>>>> + return 0;
>>>
>>> You drop the errors here. Better refactor the code to a scheme like this:
>>>
>> In msix_fire_vector_notifier(), there is "assert(ret >= 0);". But I
>> think ivshmem can work even if irqfd can not be established, and fall
>> back to the origin scheme. So here just ignore err. Is it proper?
>
> If you have an error here, something seriously went wrong. So better let
> the user know than falling back to "slow" mode silently. That's what
> other users do as well.
>
Refer to vfio_msix_vector_use(), I think to avoid silently, we can
error_report() to notify user. And because as msi_route is limited
resource, so we should hold with fail, and keep the Qemu still in
work.
Is it OK?
>>
>>> err = service();
>>> if (err) {
>>> roll_back();
>>> return err;
>>> /* or: goto roll_back_... */
>>> }
>>>
>>>> +}
>>>> +
[...]
>>
>> And I think, this is the way to push interrupt subsystem out of big
>> lock for KVM. For TCG code, we can use something like
>> kvm_irqchip_add_msi_route(). How do you think about it?
>
> Cannot follow what your idea is.
>
I mean to add RCU style interrupt dispatching table for TCG.
> Also not that MSI on x86 is special anyway as it is not routed in any
> user-space device model (so far - emulated interrupt remapping will
> change this) but goes directly to the target VCPU via the corresponding
> IOCTL.
>
Oh, yes, MSI is so special. The user space dispatch table can only
shortcut IRQ to something like "ioapic", not directly to vcpu
Regards,
Pingfan
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux
