On 11/23/2010 08:35 AM, Yang, Sheng wrote:
On Tuesday 23 November 2010 14:17:28 Avi Kivity wrote: > 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. Current we don't support LM with VT-d...
Isn't this work useful for virtio 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? We still want QEmu to cover a part of entries which hasn't been enabled yet(which won't existed in routing table), but kernel would cover all mask bit regardless of if it's enabled. So QEmu can query any entry to check the maskbit, but not address/data.
Don't understand. If we support reading/writing entire entries, that works for both enabled and disabled entries?
> What about the pending bits? We didn't cover it here - and it's in another MMIO space(PBA). Of course we can add more flags for it later.
When an entry is masked, we need to set the pending bit for it somewhere. I guess this is broken in the existing code (without your patches)?
> > Also need a new exit reason to tell userspace that an msix entry has > changed, so userspace can update mappings. I think we don't need it. Whenever userspace want to get one mapping which is an enabled MSI-X entry, it can check it with the API above(which is quite rare, because kernel would handle all of them when guest is accessing them). If it's a disabled entry, the context inside userspace MMIO record is the correct one(and only one). The only place I think QEmu need to sync is when MSI-X is about to disabled, QEmu need to update it's own MMIO record.
So in-kernel handling of mmio would be decided per entry? I'm trying to simplify this, and simplest thing is - all or nothing.
-- 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