Re: Mask bit support's API

Michael S. Tsirkin
2010-12-02T14:27:03Z

On Thu, Dec 02, 2010 at 03:56:52PM +0200, Avi Kivity wrote:
> On 12/02/2010 03:47 PM, Michael S. Tsirkin wrote:
> >>
> >> Which case? the readl() doesn't need access to the routing table,
> >> just the entry.
> >
> >One thing that read should do is flush in the outstanding
> >interrupts and flush out the mask bit writes.
>
> The mask bit writes are synchronous.
>
> wrt interrupts, we can deal with assigned devices, and can poll
> irqfds. But we can't force vhost-net to issue an interrupt (and I
> don't think it's required).

To clarify:

mask write
read

it is safe for guest to assume no more interrupts

where as with a simple
mask write

an interrupt might be in flight and get delivered shortly afterwards.

> >> Oh, I think there is a terminology problem, I was talking about
> >> kvm's irq routing table, you were talking about the msix entries.
> >>
> >> I think treating it as a cache causes more problems, since there are
> >> now two paths for reads (in cache and not in cache) and more things
> >> for writes to manage.
> >>
> >> Here's my proposed API:
> >>
> >> KVM_DEFINE_MSIX_TABLE(table_id, nr_entries, msix_base_gpa,
> >> pending_bitmap_base_gpa)
> >>
> >> - called when the guest enables msix
> >
> >I would add virtual addresses so that we can use swappable memory to
> >store the state.
>
> Right.
>
> >If we do, maybe we can just keep the table there and then
> >KVM_SET/GET_MSIX_ENTRY and the new exit won't be needed?
>
> Still need to to let userspace know it needs to reprogram the irqfd
> or whatever it uses to inject the interrupt.

Why do we need to reprogram irqfd? I thought irqfd would map to an
entry within the table instead of address/data as now.
Could you clarify please?

> >> KVM_REMOVE_MSIX_TABLE(table_id)
> >>
> >> - called when the guest disables msix
> >>
> >> KVM_SET_MSIX_ENTRY(table_id, entry_id, contents)
> >>
> >> - called when the guest enables msix (to initialize it), or after
> >> live migration
> >
> >What is entry_id here?
>
> Entry within the table.

So I think KVM_DEFINE_MSIX_TABLE should be called when msix is
enabled (note: it can not be called at boot anyway since pa
depends on BAR assigned by BIOS).

> >> Michael? I think that should work for virtio and vfio assigned
> >> devices? Not sure about pending bits.
> >
> >Pending bits must be tracked in kernel, but I don't see
> >how we can support polling mode if we don't exit to userspace
> >on pending bit reads.
> >
> >This does mean that some reads will be fast and some will be
> >slow, and it's a bit sad that we seem to be optimizing
> >for specific guests, but I just can't come up with
> >anything better.
> >
>
> If the pending bits live in userspace memory, the device model can
> update them directly?

Note that these are updated on an interrupt, so updating them
in userspace would need get_user_page etc trickery,
and add the overhead of atomics.

Further I think it's important to avoid the overhead of updating them
all the time, and only do this when an interrupt is
masked or on pending bits read. Since userspace does not know
when interrupts are masked, this means do update on each read.

> >So maybe just add an ioctl to get and to clear pending bits.
> >Maybe set for symmetry.
>
> For live migration too. But if they live in memory, no need for
> get/set, just specify the address.
>
> --
> error compiling committee.c: too many arguments to function