Re: Mask bit support's API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux