lkml.org 
[lkml]   [2021]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [patch 21/32] NTB/msi: Convert to msi_on_each_desc()
    Date
    On Thu, Dec 09 2021 at 23:09, Thomas Gleixner wrote:
    > On Thu, Dec 09 2021 at 16:58, Jason Gunthorpe wrote:
    >> Okay, I think I get it. Would be nice to have someone from intel
    >> familiar with the vIOMMU protocols and qemu code remark what the
    >> hypervisor side can look like.
    >>
    >> There is a bit more work here, we'd have to change VFIO to somehow
    >> entirely disconnect the kernel IRQ logic from the MSI table and
    >> directly pass control of it to the guest after the hypervisor IOMMU IR
    >> secures it. ie directly mmap the msi-x table into the guest
    >
    > That makes everything consistent and a clear cut on all levels, right?

    Let me give a bit more rationale here, why I think this is the right
    thing to do. There are several problems with IMS both on the host and on
    the guest side:

    1) Contrary to MSI/MSI-X the address/data pair is not completely
    managed by the core. It's handed off to driver writers in the
    hope they get it right.

    2) Without interrupt remapping there is a fundamental issue on x86
    for the affinity setting case, as there is no guarantee that
    the magic protocol which we came up with (see msi_set_affinity()
    in the x86 code) is correctly implemented at the driver level or
    that the update is truly atomic so that the problem does not
    arise. My interrest in chasing these things is exactly zero.

    With interrupt remapping the affinity change happens at the IRTE
    level and not at the device level. It's a one time setup for the
    device.

    Just for the record:

    The ATH11 thing does not have that problem by pure luck because
    multi-vector MSI is not supported on X86 unless interrupt
    remapping is enabled.

    The switchtec NTB thing will fall apart w/o remapping AFAICT.

    3) With remapping the message for the device is constructed at
    allocation time. It does not change after that because the affinity
    change happens at the remapping level, which eliminates #2 above.

    That has another advantage for IMS because it does not require any
    synchronization with the queue or whatever is involved. The next
    interrupt after the change at the remapping level ends up on the
    new target.

    4) For the guest side we agreed that we need an hypercall because the
    host can't trap the write to the MSI[-X] entry anymore.

    Aside of the fact that this creates a special case for IMS which is
    undesirable in my opinion, it's not really obvious where the
    hypercall should be placed to work for all scenarios so that it can
    also solve the existing issue of silent failures.

    5) It's not possible for the kernel to reliably detect whether it is
    running on bare metal or not. Yes we talked about heuristics, but
    that's something I really want to avoid.

    When looking at the above I came to the conclusion that the consistent
    way is to make IMS depend on IR both on the host and the guest as this
    solves all of the above in one go.

    How would that work? With IR the irqdomain hierarchy looks like this:

    |--IO/APIC
    |--MSI
    vector -- IR --|--MIX-X
    |--IMS

    There are several context where this matters:

    1) Allocation of an interrupt, e.g. pci_alloc_irq_vectors().

    2) Activation of an interrupt which happens during allocation and/or
    at request_irq() time

    3) Interrupt affinity setting

    #1 Allocation

    That allocates an IRTE, which can fail

    #2 Activation

    That's the step where actually a CPU vector is allocated, where the
    IRTE is updated and where the device message is composed to target
    the IRTE.

    On X86 activation is happening twice:

    1) During allocation it allocates a special CPU vector which is
    handed out to all allocated interrupts. That's called reservation
    mode. This was introduced to prevent vector exhaustion for two
    cases:

    - Devices allocating tons of MSI-X vectors without using
    them. That obviously needs to be fixed at the device driver
    level, but due to the fact that post probe() allocation is not
    supported, that's not always possible

    - CPU hotunplug

    All vectors targeting the outgoing CPU need to be migrated to a
    new target CPU, which can result in exhaustion of the vector
    space.

    Reservation mode avoids that because it just uses a unique
    vector for all interrupts which are allocated but not
    requested.

    2) On request_irq()

    As the vector assigned during allocation is just a place holder
    to make the MSI hardware happy it needs to be replaced by a
    real vector.

    Both can fail and the error is propagated through the call chain

    #3 Changing the interrupt affinity

    This obviously needs to allocate a new target CPU vector and update
    the IRTE.

    Allocating a new target CPU vector can fail.

    When looking at it from the host side, then the host needs to do the
    same things:

    1) Allocate an IRTE for #1

    2) Update the IRTE for #2 and #3

    But that does not necessarily mean that we need two hypercalls. We can
    get away with one in the code which updates the IRTE and that would be
    the point where the host side has to allocate the backing host
    interrupt, which would replace that allocate on unmask mechanism which
    is used today.

    It might look awkward on first sight that an IRTE update can fail, but
    it's not that awkward when put into context:

    The first update happens during activation and activation can fail for
    various reasons.

    The charm is that his works for everything from INTx to IMS because all
    of them go through the same procedure, except that INTx (IO/APIC) does
    not support the reservation mode dance.

    Thoughts?

    Thanks,

    tglx

    \
     
     \ /
      Last update: 2021-12-10 01:27    [W:5.422 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site