Re: Mask bit support's API

Avi Kivity
2010-11-23T06:17:33Z

On 11/23/2010 08:09 AM, Yang, Sheng wrote:
> Hi Avi,
>
> I've purposed the following API for mask bit support.
>
> The main point is, QEmu can know which entries are enabled(by pci_enable_msix()).
> And for enabled entries, kernel own it, including MSI data/address and mask
> bit(routing table and mask bitmap). QEmu should use KVM_GET_MSIX_ENTRY ioctl to
> get them(and it can sync with them if it want to do so).
>
> Before entries are enabled, QEmu can still use it's own MSI table(because we
> didn't contain these kind of information in kernel, and it's unnecessary for
> kernel).
>
> The KVM_MSIX_FLAG_ENTRY flag would be clear if QEmu want to query one entry didn't
> exist in kernel - or we can simply return -EINVAL for it.
>
> I suppose it would be rare for QEmu to use this interface to get the context of
> entry(the only case I think is when MSI-X disable and QEmu need to sync the
> context), so performance should not be an issue.
>
> What's your opinion?
>
> > #define KVM_GET_MSIX_ENTRY _IOWR(KVMIO, 0x7d, struct kvm_msix_entry)

Need SET_MSIX_ENTRY for live migration as well.

> > #define KVM_UPDATE_MSIX_MMIO _IOW(KVMIO, 0x7e, struct kvm_msix_mmio)
> >
> > #define KVM_MSIX_TYPE_ASSIGNED_DEV 1
> >
> > #define KVM_MSIX_FLAG_MASKBIT (1<< 0)
> > #define KVM_MSIX_FLAG_QUERY_MASKBIT (1<< 0)
> > #define KVM_MSIX_FLAG_ENTRY (1<< 1)
> > #define KVM_MSIX_FLAG_QUERY_ENTRY (1<< 1)
> >

Why is there a need for the flag? If we simply get/set entire entries,
that includes the mask bits?

What about the pending bits?

> > struct kvm_msix_entry {
> > __u32 id;
> > __u32 type;
> > __u32 entry; /* The index of entry in the MSI-X table */
> > __u32 flags;
> > __u32 query_flags;
> > union {
> > struct {
> > __u32 addr_lo;
> > __u32 addr_hi;
> > __u32 data;

Isn't the mask bit in the last word? Or maybe I'm confused about the
format.

>
> > } msi_entry;
> > __u32 reserved[12];
> > };
> > };
> >
> > #define KVM_MSIX_MMIO_FLAG_REGISTER (1<< 0)
> > #define KVM_MSIX_MMIO_FLAG_UNREGISTER (1<< 1)
> > #define KVM_MSIX_MMIO_FLAG_MASK 0x3
> >
> > struct kvm_msix_mmio {
> > __u32 id;
> > __u32 type;
> > __u64 base_addr;
> > __u32 max_entries_nr;
> > __u32 flags;
> > __u32 reserved[6];
> > };

Also need a new exit reason to tell userspace that an msix entry has
changed, so userspace can update mappings.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.